date:20190909

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

2019-09-09 Thread 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] D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test

2019-09-09 Thread 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

2019-09-09 Thread 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

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

2019-09-09 Thread 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&view=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&r1=371475&r2=371476&view=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&r1=371475&r2=371476&view=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] D66796: [clang] Loop pragma vectorize(disable)

2019-09-09 Thread 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

fixing 2 typos

2019-09-09 Thread 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] D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test

2019-09-09 Thread 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

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

2019-09-09 Thread 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&view=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&r1=371477&r2=371478&view=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&r1=371477&r2=371478&view=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: cfe/

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

2019-09-09 Thread 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

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

2019-09-09 Thread 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) {
+  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

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

2019-09-09 Thread 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&view=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&r1=359366&r2=359367&view=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 &LV) {
> -  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 &LV) {
> +  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 &Ctx, const Expr *Arg) {
> +static bool EvaluateBuiltinConstantP(EvalInfo &Info, 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;
> +

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

2019-09-09 Thread 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

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

2019-09-09 Thread 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

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

2019-09-09 Thread 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&view=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&r1=371483&r2=371484&view=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&r1=371483&r2=371484&view=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&r1=371483&r2=371484&view=diff
==
--- cfe/trunk/lib/CodeGen/Cod

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

2019-09-09 Thread 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

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

2019-09-09 Thread 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&id=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 &Msg)
+: DiagnosticInfoWithLocationBase(DK_MisExpect, DS_Warning,
+ *Inst->getParent()->getParent(),
+ Inst->getDebugLoc()),
+  Msg(Msg) {}
+
+void DiagnosticInfoMisExpect::print(DiagnosticPrinter &DP) 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

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

2019-09-09 Thread 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 &PreviousBase : 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

r371488 - Revert "clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM"

2019-09-09 Thread Petr Hosek via cfe-commits

Author: phosek
Date: Mon Sep  9 23:25:13 2019
New Revision: 371488

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

This reverts commit r371484: this broke sanitizer-x86_64-linux-fast bot.

Removed:
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=371488&r1=371487&r2=371488&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Mon Sep  9 
23:25:13 2019
@@ -275,12 +275,7 @@ 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=371488&r1=371487&r2=371488&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Sep  9 23:25:13 2019
@@ -1042,7 +1042,6 @@ 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=371488&r1=371487&r2=371488&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Mon Sep  9 23:25:13 2019
@@ -14,7 +14,6 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclGroup.h"
-#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LangStandard.h"
 #include "clang/Basic/SourceManager.h"
@@ -366,9 +365,6 @@ namespace clang {
 bool StackSizeDiagHandler(const llvm::DiagnosticInfoStackSize &D);
 /// Specialized handler for unsupported backend feature diagnostic.
 void UnsupportedDiagHandler(const llvm::DiagnosticInfoUnsupported &D);
-/// Specialized handler for misexpect warnings.
-/// Note that misexpect remarks are emitted through ORE
-void MisExpectDiagHandler(const llvm::DiagnosticInfoMisExpect &D);
 /// Specialized handlers for optimization remarks.
 /// Note that these handlers only accept remarks and they always handle
 /// them.
@@ -621,25 +617,6 @@ void BackendConsumer::UnsupportedDiagHan
 << Filename << Line << Column;
 }
 
-void BackendConsumer::MisExpectDiagHandler(
-const llvm::DiagnosticInfoMisExpect &D) {
-  StringRef Filename;
-  unsigned Line, Column;
-  bool BadDebugInfo = false;
-  FullSourceLoc Loc =
-  getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, Line, Column);
-
-  Diags.Report(Loc, diag::warn_profile_data_misexpect) << D.getMsg().str();
-
-  if (BadDebugInfo)
-// If we were not able to translate the file:line:col information
-// back to a SourceLocation, at least emit a note stating that
-// we could not translate this location. This can happen in the
-// case of #line di

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

2019-09-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

In D65433#1661865 , @arphaman wrote:

> When fully implemented, will define inline tweak work with C++ methods in 
> classes as well?


Yes you can see an example of it in the tests provided in `TweakTests.cpp`




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"; }
+

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.


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

[PATCH] D67292: [clang-tidy] Fix bug in bugprone-use-after-move check

2019-09-09 Thread Martin Böhme via Phabricator via cfe-commits

mboehme added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/bugprone-use-after-move.cpp:1198
+}
+  }
   for (int i = 0; i < 10; ++i) {

gribozavr wrote:
> ymandel wrote:
> > gribozavr wrote:
> > > Unless you think it is redundant, could you also add
> > > 
> > > ```
> > > if (A a1; A(std::move(a2)).getInt() > 0) {}
> > > ```
> > > 
> > > Also some true positive tests would be good:
> > > 
> > > ```
> > > if (A a1; A(std::move(a2)).getInt() > A(std::move(a2)).getInt()) {}
> > > ```
> > > 
> > > ```
> > > A a1;
> > > if (A a2 = std::move(a1); A(std::move(a1)) > 0) {}
> > > ```
> > Done, but any idea why everything in this function is placed inside a loop? 
> >  Looks like its just for scoping, but then why not just a compound 
> > statement, as is done above? This feels very odd.
> I think it is to ensure that the checker understands the sequencing. If it 
> didn't, then the loop would trigger the "moved twice" logic.
(Original author of the test here.)

Correct. I should probably have added a comment explaining this when I wrote 
the test. Feel free to add such a comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67292



___
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

2019-09-09 Thread Sam McCall via Phabricator via cfe-commits

sammccall 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"; }
+

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)


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

[PATCH] D67274: [clangd] Improve output of semantic highlighting tests in case of failures

2019-09-09 Thread Haojian Wu via Phabricator via cfe-commits

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

nice, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67274



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

[PATCH] D67277: [clangd] Replace HighlightingKind::NumKinds with LastKind. NFC

2019-09-09 Thread Haojian Wu via Phabricator via cfe-commits

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

LGTM, though I think the current implementation is a common pattern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67277



___
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

2019-09-09 Thread Ilya Biryukov via Phabricator via cfe-commits

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

- Functions to compute highlighting kinds for things are separated from the 
ones that add highlighting tokens. This keeps each of them more focused on what 
they're doing: getting locations and figuring out the kind of the entity, 
correspondingly.

- Less special cases in visitor for various nodes.

As a side effect, we started to highlight 'auto' for lambdas as having
class type.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67341

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  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
@@ -152,7 +152,7 @@
 $Primitive[[auto]] $LocalVariable[[VeryLongVariableName]] = 12312;
 $Class[[AS]] $LocalVariable[[AA]];
 $Primitive[[auto]] $LocalVariable[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
-auto $LocalVariable[[FN]] = [ $LocalVariable[[AA]]]($Primitive[[int]] $Parameter[[A]]) -> $Primitive[[void]] {};
+$Class[[auto]] $LocalVariable[[FN]] = [ $LocalVariable[[AA]]]($Primitive[[int]] $Parameter[[A]]) -> $Primitive[[void]] {};
 $LocalVariable[[FN]](12312);
   }
 )cpp",
@@ -370,7 +370,7 @@
   $Enum[[auto]] &$Variable[[AER]] = $Variable[[AE]];
   $Primitive[[auto]] $Variable[[Form]] = 10.2 + 2 * 4;
   $Primitive[[decltype]]($Variable[[Form]]) $Variable[[F]] = 10;
-  auto $Variable[[Fun]] = []()->$Primitive[[void]]{};
+  $Class[[auto]] $Variable[[Fun]] = []()->$Primitive[[void]]{};
 )cpp",
   R"cpp(
   class $Class[[G]] {};
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(DeclRefExp

[PATCH] D67277: [clangd] Replace HighlightingKind::NumKinds with LastKind. NFC

2019-09-09 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In D67277#1662669 , @hokein wrote:

> LGTM, though I think the current implementation is a common pattern.


Thanks.

It's indeed a common pattern. The problem is that it forces clients to handle 
this extra case in every switch statement (otherwise we get compiler warnings).
`LastKind` avoids this, therefore it's arguably a little better for the clients.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67277



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

[clang-tools-extra] r371373 - [clangd] Improve output of semantic highlighting tests in case of failures

2019-09-09 Thread Ilya Biryukov via cfe-commits

Author: ibiryukov
Date: Mon Sep  9 01:47:05 2019
New Revision: 371373

URL: http://llvm.org/viewvc/llvm-project?rev=371373&view=rev
Log:
[clangd] Improve output of semantic highlighting tests in case of failures

Summary:
Instead of matching lists of highlightings, we annotate input code with
resulting highlightings and diff it against the expected annotated input.

In case of failures, this produces much nicer output in form of text-based
diffs.

Reviewers: hokein

Reviewed By: hokein

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
clang-tools-extra/trunk/clangd/SemanticHighlighting.h
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=371373&r1=371372&r2=371373&view=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp Mon Sep  9 01:47:05 
2019
@@ -351,6 +351,43 @@ takeLine(ArrayRef All
 }
 } // namespace
 
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingKind K) {
+  switch (K) {
+  case HighlightingKind::Variable:
+return OS << "Variable";
+  case HighlightingKind::LocalVariable:
+return OS << "LocalVariable";
+  case HighlightingKind::Parameter:
+return OS << "Parameter";
+  case HighlightingKind::Function:
+return OS << "Function";
+  case HighlightingKind::Method:
+return OS << "Method";
+  case HighlightingKind::StaticMethod:
+return OS << "StaticMethod";
+  case HighlightingKind::Field:
+return OS << "Field";
+  case HighlightingKind::StaticField:
+return OS << "StaticField";
+  case HighlightingKind::Class:
+return OS << "Class";
+  case HighlightingKind::Enum:
+return OS << "Enum";
+  case HighlightingKind::EnumConstant:
+return OS << "EnumConstant";
+  case HighlightingKind::Namespace:
+return OS << "Namespace";
+  case HighlightingKind::TemplateParameter:
+return OS << "TemplateParameter";
+  case HighlightingKind::Primitive:
+return OS << "Primitive";
+  case HighlightingKind::Macro:
+return OS << "Macro";
+  case HighlightingKind::NumKinds:
+llvm_unreachable("NumKinds is not a valid HighlightingKind");
+  }
+}
+
 std::vector
 diffHighlightings(ArrayRef New,
   ArrayRef Old) {

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.h?rev=371373&r1=371372&r2=371373&view=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.h (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.h Mon Sep  9 01:47:05 
2019
@@ -18,6 +18,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SEMANTICHIGHLIGHTING_H
 
 #include "Protocol.h"
+#include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace clangd {
@@ -42,6 +43,7 @@ enum class HighlightingKind {
 
   NumKinds,
 };
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingKind K);
 
 // Contains all information needed for the highlighting a token.
 struct HighlightingToken {

Modified: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp?rev=371373&r1=371372&r2=371373&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp Mon 
Sep  9 01:47:05 2019
@@ -10,9 +10,14 @@
 #include "ClangdServer.h"
 #include "Protocol.h"
 #include "SemanticHighlighting.h"
+#include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -59,6 +64,36 @@ std::vector getExpect
   return ExpectedTokens;
 }
 
+/// Annotates the input code with provided semantic highlightings. Results look
+/// something like:
+///   class $Class[[X]] {
+/// $Primitive[[int]] $Field[[a]] = 0;
+///   };
+std::string annotate(llvm::StringRef Input,
+ llvm::ArrayRef Tokens) {
+  assert(std::is_sorted(
+  Tokens.begin(), Tokens.end(),
+  [](const HighlightingToken &L, const HighlightingToken &R) {
+return L.R.start < R.R.start;
+  }));
+
+  std::string Result;
+  unsigned NextChar = 0;
+  for (auto &T : Tokens) {
+unsigned StartOffset = llvm::cantF

[PATCH] D67274: [clangd] Improve output of semantic highlighting tests in case of failures

2019-09-09 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371373: [clangd] Improve output of semantic highlighting 
tests in case of failures (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67274?vs=219098&id=219294#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67274

Files:
  clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
  clang-tools-extra/trunk/clangd/SemanticHighlighting.h
  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
@@ -351,6 +351,43 @@
 }
 } // namespace
 
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingKind K) {
+  switch (K) {
+  case HighlightingKind::Variable:
+return OS << "Variable";
+  case HighlightingKind::LocalVariable:
+return OS << "LocalVariable";
+  case HighlightingKind::Parameter:
+return OS << "Parameter";
+  case HighlightingKind::Function:
+return OS << "Function";
+  case HighlightingKind::Method:
+return OS << "Method";
+  case HighlightingKind::StaticMethod:
+return OS << "StaticMethod";
+  case HighlightingKind::Field:
+return OS << "Field";
+  case HighlightingKind::StaticField:
+return OS << "StaticField";
+  case HighlightingKind::Class:
+return OS << "Class";
+  case HighlightingKind::Enum:
+return OS << "Enum";
+  case HighlightingKind::EnumConstant:
+return OS << "EnumConstant";
+  case HighlightingKind::Namespace:
+return OS << "Namespace";
+  case HighlightingKind::TemplateParameter:
+return OS << "TemplateParameter";
+  case HighlightingKind::Primitive:
+return OS << "Primitive";
+  case HighlightingKind::Macro:
+return OS << "Macro";
+  case HighlightingKind::NumKinds:
+llvm_unreachable("NumKinds is not a valid HighlightingKind");
+  }
+}
+
 std::vector
 diffHighlightings(ArrayRef New,
   ArrayRef Old) {
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
@@ -10,9 +10,14 @@
 #include "ClangdServer.h"
 #include "Protocol.h"
 #include "SemanticHighlighting.h"
+#include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -59,6 +64,36 @@
   return ExpectedTokens;
 }
 
+/// Annotates the input code with provided semantic highlightings. Results look
+/// something like:
+///   class $Class[[X]] {
+/// $Primitive[[int]] $Field[[a]] = 0;
+///   };
+std::string annotate(llvm::StringRef Input,
+ llvm::ArrayRef Tokens) {
+  assert(std::is_sorted(
+  Tokens.begin(), Tokens.end(),
+  [](const HighlightingToken &L, const HighlightingToken &R) {
+return L.R.start < R.R.start;
+  }));
+
+  std::string Result;
+  unsigned NextChar = 0;
+  for (auto &T : Tokens) {
+unsigned StartOffset = llvm::cantFail(positionToOffset(Input, T.R.start));
+unsigned EndOffset = llvm::cantFail(positionToOffset(Input, T.R.end));
+assert(StartOffset <= EndOffset);
+assert(NextChar <= StartOffset);
+
+Result += Input.substr(NextChar, StartOffset - NextChar);
+Result += llvm::formatv("${0}[[{1}]]", T.Kind,
+Input.substr(StartOffset, EndOffset - StartOffset));
+NextChar = EndOffset;
+  }
+  Result += Input.substr(NextChar);
+  return Result;
+}
+
 void checkHighlightings(llvm::StringRef Code,
 std::vector>
@@ -68,8 +103,8 @@
   for (auto File : AdditionalFiles)
 TU.AdditionalFiles.insert({File.first, File.second});
   auto AST = TU.build();
-  std::vector ActualTokens = getSemanticHighlightings(AST);
-  EXPECT_THAT(ActualTokens, getExpectedTokens(Test)) << Code;
+
+  EXPECT_EQ(Code, annotate(Test.code(), getSemanticHighlightings(AST)));
 }
 
 // Any annotations in OldCode and NewCode are converted into their corresponding
Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.h
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.h
@@ -18,6 +18,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SEMANTICHIGHLIGHTING_H
 
 #include "Protocol.h"
+#include "llvm/Support/raw_ostream.h"
 
 namespace clang {
 namespace clangd {
@@ -42,6 +43,7 @@

Re: r369705 - [Clang][CodeGen] set alias linkage on QualType

2019-09-09 Thread Hans Wennborg via cfe-commits

Merged to release_90 in r371372 (along with the follow-up test fix).

On Thu, Aug 22, 2019 at 10:45 PM Nick Desaulniers via cfe-commits
 wrote:
>
> Author: nickdesaulniers
> Date: Thu Aug 22 13:47:12 2019
> New Revision: 369705
>
> URL: http://llvm.org/viewvc/llvm-project?rev=369705&view=rev
> Log:
> [Clang][CodeGen] set alias linkage on QualType
>
> Summary:
> It seems that CodeGen was always using ExternalLinkage when emitting a
> GlobalDecl with __attribute__((alias)). This leads to symbol
> redefinitions (ODR) that cause failures at link time for static aliases.
> This is readily attempting to link an ARM (32b) allyesconfig Linux
> kernel built with Clang.
>
> Reported-by: nathanchance
> Suggested-by: ihalip
> Link: https://bugs.llvm.org/show_bug.cgi?id=42377
> Link: https://github.com/ClangBuiltLinux/linux/issues/631
>
> Reviewers: rsmith, aaron.ballman, erichkeane
>
> Reviewed By: aaron.ballman
>
> Subscribers: javed.absar, kristof.beyls, cfe-commits, srhines, ihalip, 
> nathanchance
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D66492
>
> Modified:
> cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> cfe/trunk/test/CodeGen/alias.c
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=369705&r1=369704&r2=369705&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Aug 22 13:47:12 2019
> @@ -4363,17 +4363,22 @@ void CodeGenModule::EmitAliasDefinition(
>// Create a reference to the named value.  This ensures that it is emitted
>// if a deferred decl.
>llvm::Constant *Aliasee;
> -  if (isa(DeclTy))
> +  llvm::GlobalValue::LinkageTypes LT;
> +  if (isa(DeclTy)) {
>  Aliasee = GetOrCreateLLVMFunction(AA->getAliasee(), DeclTy, GD,
>/*ForVTable=*/false);
> -  else
> +LT = getFunctionLinkage(GD);
> +  } else {
>  Aliasee = GetOrCreateLLVMGlobal(AA->getAliasee(),
>  llvm::PointerType::getUnqual(DeclTy),
>  /*D=*/nullptr);
> +LT = getLLVMLinkageVarDefinition(cast(GD.getDecl()),
> + D->getType().isConstQualified());
> +  }
>
>// Create the new alias itself, but don't set a name yet.
> -  auto *GA = llvm::GlobalAlias::create(
> -  DeclTy, 0, llvm::Function::ExternalLinkage, "", Aliasee, &getModule());
> +  auto *GA =
> +  llvm::GlobalAlias::create(DeclTy, 0, LT, "", Aliasee, &getModule());
>
>if (Entry) {
>  if (GA->getAliasee() == Entry) {
>
> Modified: cfe/trunk/test/CodeGen/alias.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/alias.c?rev=369705&r1=369704&r2=369705&view=diff
> ==
> --- cfe/trunk/test/CodeGen/alias.c (original)
> +++ cfe/trunk/test/CodeGen/alias.c Thu Aug 22 13:47:12 2019
> @@ -2,6 +2,7 @@
>  // RUN: %clang_cc1 -triple i386-pc-linux-gnu -emit-llvm -o - %s | FileCheck 
> -check-prefix=CHECKBASIC %s
>  // RUN: %clang_cc1 -triple armv7a-eabi -mfloat-abi hard -emit-llvm -o - %s | 
> FileCheck -check-prefix=CHECKCC %s
>  // RUN: %clang_cc1 -triple armv7a-eabi -mfloat-abi hard -S -o - %s | 
> FileCheck -check-prefix=CHECKASM %s
> +// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefix=CHECKGLOBALS 
> %s
>
>  int g0;
>  // CHECKBASIC-DAG: @g0 = common global i32 0
> @@ -88,3 +89,13 @@ void test8_zed() __attribute__((alias("t
>  void test9_bar(void) { }
>  void test9_zed(void) __attribute__((section("test")));
>  void test9_zed(void) __attribute__((alias("test9_bar")));
> +
> +// Test that the alias gets its linkage from its declared qual type.
> +// CHECKGLOBALS: @test10_foo = internal
> +// CHECKGLOBALS-NOT: @test10_foo = dso_local
> +int test10;
> +static int test10_foo __attribute__((alias("test10")));
> +// CHECKGLOBALS: @test11_foo = internal
> +// CHECKGLOBALS-NOT: @test11_foo = dso_local
> +void test11(void) {}
> +static void test11_foo(void) __attribute__((alias("test11")));
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r371375 - [clangd] Replace HighlightingKind::NumKinds with LastKind. NFC

2019-09-09 Thread Ilya Biryukov via cfe-commits

Author: ibiryukov
Date: Mon Sep  9 01:57:17 2019
New Revision: 371375

URL: http://llvm.org/viewvc/llvm-project?rev=371375&view=rev
Log:
[clangd] Replace HighlightingKind::NumKinds with LastKind. NFC

Summary:
The latter simplifies the client code by avoiding the need to handle it
as a separate case statement.

Reviewers: hokein

Reviewed By: hokein

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
clang-tools-extra/trunk/clangd/SemanticHighlighting.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=371375&r1=371374&r2=371375&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Mon Sep  9 01:57:17 2019
@@ -87,7 +87,7 @@ CompletionItemKindBitset defaultCompleti
 std::vector> buildHighlightScopeLookupTable() {
   std::vector> LookupTable;
   // HighlightingKind is using as the index.
-  for (int KindValue = 0; KindValue < (int)HighlightingKind::NumKinds;
+  for (int KindValue = 0; KindValue <= (int)HighlightingKind::LastKind;
++KindValue)
 LookupTable.push_back({toTextMateScope((HighlightingKind)(KindValue))});
   return LookupTable;

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp?rev=371375&r1=371374&r2=371375&view=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp Mon Sep  9 01:57:17 
2019
@@ -383,9 +383,8 @@ llvm::raw_ostream &operator<<(llvm::raw_
 return OS << "Primitive";
   case HighlightingKind::Macro:
 return OS << "Macro";
-  case HighlightingKind::NumKinds:
-llvm_unreachable("NumKinds is not a valid HighlightingKind");
   }
+  llvm_unreachable("invalid HighlightingKind");
 }
 
 std::vector
@@ -511,8 +510,6 @@ llvm::StringRef toTextMateScope(Highligh
 return "storage.type.primitive.cpp";
   case HighlightingKind::Macro:
 return "entity.name.function.preprocessor.cpp";
-  case HighlightingKind::NumKinds:
-llvm_unreachable("must not pass NumKinds to the function");
   }
   llvm_unreachable("unhandled HighlightingKind");
 }

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.h?rev=371375&r1=371374&r2=371375&view=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.h (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.h Mon Sep  9 01:57:17 
2019
@@ -41,7 +41,7 @@ enum class HighlightingKind {
   Primitive,
   Macro,
 
-  NumKinds,
+  LastKind = Macro
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingKind K);
 


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

[PATCH] D67277: [clangd] Replace HighlightingKind::NumKinds with LastKind. NFC

2019-09-09 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371375: [clangd] Replace HighlightingKind::NumKinds with 
LastKind. NFC (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67277?vs=219102&id=219296#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67277

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
  clang-tools-extra/trunk/clangd/SemanticHighlighting.h


Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -87,7 +87,7 @@
 std::vector> buildHighlightScopeLookupTable() {
   std::vector> LookupTable;
   // HighlightingKind is using as the index.
-  for (int KindValue = 0; KindValue < (int)HighlightingKind::NumKinds;
+  for (int KindValue = 0; KindValue <= (int)HighlightingKind::LastKind;
++KindValue)
 LookupTable.push_back({toTextMateScope((HighlightingKind)(KindValue))});
   return LookupTable;
Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.h
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.h
@@ -41,7 +41,7 @@
   Primitive,
   Macro,
 
-  NumKinds,
+  LastKind = Macro
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingKind K);
 
Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -383,9 +383,8 @@
 return OS << "Primitive";
   case HighlightingKind::Macro:
 return OS << "Macro";
-  case HighlightingKind::NumKinds:
-llvm_unreachable("NumKinds is not a valid HighlightingKind");
   }
+  llvm_unreachable("invalid HighlightingKind");
 }
 
 std::vector
@@ -511,8 +510,6 @@
 return "storage.type.primitive.cpp";
   case HighlightingKind::Macro:
 return "entity.name.function.preprocessor.cpp";
-  case HighlightingKind::NumKinds:
-llvm_unreachable("must not pass NumKinds to the function");
   }
   llvm_unreachable("unhandled HighlightingKind");
 }


Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -87,7 +87,7 @@
 std::vector> buildHighlightScopeLookupTable() {
   std::vector> LookupTable;
   // HighlightingKind is using as the index.
-  for (int KindValue = 0; KindValue < (int)HighlightingKind::NumKinds;
+  for (int KindValue = 0; KindValue <= (int)HighlightingKind::LastKind;
++KindValue)
 LookupTable.push_back({toTextMateScope((HighlightingKind)(KindValue))});
   return LookupTable;
Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.h
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.h
@@ -41,7 +41,7 @@
   Primitive,
   Macro,
 
-  NumKinds,
+  LastKind = Macro
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingKind K);
 
Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -383,9 +383,8 @@
 return OS << "Primitive";
   case HighlightingKind::Macro:
 return OS << "Macro";
-  case HighlightingKind::NumKinds:
-llvm_unreachable("NumKinds is not a valid HighlightingKind");
   }
+  llvm_unreachable("invalid HighlightingKind");
 }
 
 std::vector
@@ -511,8 +510,6 @@
 return "storage.type.primitive.cpp";
   case HighlightingKind::Macro:
 return "entity.name.function.preprocessor.cpp";
-  case HighlightingKind::NumKinds:
-llvm_unreachable("must not pass NumKinds to the function");
   }
   llvm_unreachable("unhandled HighlightingKind");
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66516: [clangd] Highlight typedefs to template parameters as template parameters

2019-09-09 Thread Haojian Wu via Phabricator via cfe-commits

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

thanks for taking care of it.




Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:129
 if (const auto *TSI = TD->getTypeSourceInfo())
-  addType(TD->getLocation(), TSI->getTypeLoc().getTypePtr());
+  addType(TD->getLocation(), TSI->getTypeLoc().getType().getTypePtr());
 return true;

nit: I think the previous ` TSI->getTypeLoc().getTypePtr()` should work?



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:191
   addToken(Loc, HighlightingKind::Primitive);
+if (const TemplateTypeParmType *TD = dyn_cast(TP))
+  // TemplateTypeParmType also do not have a TagDecl.

nit: use `auto`, since the type is obvious from RHS.



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:270
+struct $Class[[A]] {
+  $Primitive[[void]] $Method[[foo]]($Class[[A]]*);
+};

Is this change relevant to this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66516



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

[PATCH] D66516: [clangd] Highlight typedefs to template parameters as template parameters

2019-09-09 Thread Ilya Biryukov via Phabricator via cfe-commits

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

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66516

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  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
@@ -474,7 +474,7 @@
 $Macro[[assert]]($Variable[[x]] != $Function[[f]]());
   }
 )cpp",
-R"cpp(
+  R"cpp(
   struct $Class[[S]] {
 $Primitive[[float]] $Field[[Value]];
 $Class[[S]] *$Field[[Next]];
@@ -488,6 +488,21 @@
 // Highlights references to BindingDecls.
 $Variable[[B1]]++;
   }
+)cpp",
+  R"cpp(
+  template
+  class $Class[[A]] {
+using $TemplateParameter[[TemplateParam1]] = $TemplateParameter[[T]];
+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]]);
+  };
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -15,6 +15,8 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
 #include 
 
 namespace clang {
@@ -128,13 +130,12 @@
 return true;
   }
 
-  bool VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc &TL) {
-// TemplateTypeParmTypeLoc does not have a TagDecl in its type ptr.
-addToken(TL.getBeginLoc(), TL.getDecl());
+  bool VisitTypedefTypeLoc(TypedefTypeLoc TL) {
+addType(TL.getBeginLoc(), TL.getTypePtr());
 return true;
   }
 
-  bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc &TL) {
+  bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) {
 if (const TemplateDecl *TD =
 TL.getTypePtr()->getTemplateName().getAsTemplateDecl())
   addToken(TL.getBeginLoc(), TD);
@@ -187,7 +188,10 @@
 if (TP->isBuiltinType())
   // Builtins must be special cased as they do not have a TagDecl.
   addToken(Loc, HighlightingKind::Primitive);
-if (const TagDecl *TD = TP->getAsTagDecl())
+if (auto *TD = dyn_cast(TP))
+  // TemplateTypeParmType also do not have a TagDecl.
+  addToken(Loc, TD->getDecl());
+if (auto *TD = TP->getAsTagDecl())
   addToken(Loc, TD);
   }
 


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -474,7 +474,7 @@
 $Macro[[assert]]($Variable[[x]] != $Function[[f]]());
   }
 )cpp",
-R"cpp(
+  R"cpp(
   struct $Class[[S]] {
 $Primitive[[float]] $Field[[Value]];
 $Class[[S]] *$Field[[Next]];
@@ -488,6 +488,21 @@
 // Highlights references to BindingDecls.
 $Variable[[B1]]++;
   }
+)cpp",
+  R"cpp(
+  template
+  class $Class[[A]] {
+using $TemplateParameter[[TemplateParam1]] = $TemplateParameter[[T]];
+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]]);
+  };
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -15,6 +15,8 @@
 #include "

[clang-tools-extra] r371379 - [clangd] Highlight typedefs to template parameters as template parameters

2019-09-09 Thread Ilya Biryukov via cfe-commits

Author: ibiryukov
Date: Mon Sep  9 02:37:17 2019
New Revision: 371379

URL: http://llvm.org/viewvc/llvm-project?rev=371379&view=rev
Log:
[clangd] Highlight typedefs to template parameters as template parameters

Summary:
Template parameters were handled outside `addType`, this led to lack of 
highlightings for typedefs
to template types.

This was never desirable, we want to highlight our typedefs as their underlying 
type.
Note that typedefs to more complicated types, like pointers and references are 
still not highlighted.

Original patch by Johan Vikström.

Reviewers: hokein, jvikstrom

Reviewed By: hokein

Subscribers: nridge, javed.absar, kristof.beyls, MaskRay, jkorous, arphaman, 
kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
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=371379&r1=371378&r2=371379&view=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp Mon Sep  9 02:37:17 
2019
@@ -15,6 +15,8 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
 #include 
 
 namespace clang {
@@ -128,13 +130,12 @@ public:
 return true;
   }
 
-  bool VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc &TL) {
-// TemplateTypeParmTypeLoc does not have a TagDecl in its type ptr.
-addToken(TL.getBeginLoc(), TL.getDecl());
+  bool VisitTypedefTypeLoc(TypedefTypeLoc TL) {
+addType(TL.getBeginLoc(), TL.getTypePtr());
 return true;
   }
 
-  bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc &TL) {
+  bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) {
 if (const TemplateDecl *TD =
 TL.getTypePtr()->getTemplateName().getAsTemplateDecl())
   addToken(TL.getBeginLoc(), TD);
@@ -187,7 +188,10 @@ private:
 if (TP->isBuiltinType())
   // Builtins must be special cased as they do not have a TagDecl.
   addToken(Loc, HighlightingKind::Primitive);
-if (const TagDecl *TD = TP->getAsTagDecl())
+if (auto *TD = dyn_cast(TP))
+  // TemplateTypeParmType also do not have a TagDecl.
+  addToken(Loc, TD->getDecl());
+if (auto *TD = TP->getAsTagDecl())
   addToken(Loc, TD);
   }
 

Modified: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp?rev=371379&r1=371378&r2=371379&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp Mon 
Sep  9 02:37:17 2019
@@ -474,7 +474,7 @@ TEST(SemanticHighlighting, GetsCorrectTo
 $Macro[[assert]]($Variable[[x]] != $Function[[f]]());
   }
 )cpp",
-R"cpp(
+  R"cpp(
   struct $Class[[S]] {
 $Primitive[[float]] $Field[[Value]];
 $Class[[S]] *$Field[[Next]];
@@ -488,6 +488,21 @@ TEST(SemanticHighlighting, GetsCorrectTo
 // Highlights references to BindingDecls.
 $Variable[[B1]]++;
   }
+)cpp",
+  R"cpp(
+  template
+  class $Class[[A]] {
+using $TemplateParameter[[TemplateParam1]] = $TemplateParameter[[T]];
+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]]);
+  };
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);


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

[PATCH] D66516: [clangd] Highlight typedefs to template parameters as template parameters

2019-09-09 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371379: [clangd] Highlight typedefs to template parameters 
as template parameters (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66516?vs=219301&id=219302#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66516

Files:
  clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
  clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp


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
@@ -474,7 +474,7 @@
 $Macro[[assert]]($Variable[[x]] != $Function[[f]]());
   }
 )cpp",
-R"cpp(
+  R"cpp(
   struct $Class[[S]] {
 $Primitive[[float]] $Field[[Value]];
 $Class[[S]] *$Field[[Next]];
@@ -488,6 +488,21 @@
 // Highlights references to BindingDecls.
 $Variable[[B1]]++;
   }
+)cpp",
+  R"cpp(
+  template
+  class $Class[[A]] {
+using $TemplateParameter[[TemplateParam1]] = $TemplateParameter[[T]];
+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]]);
+  };
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -15,6 +15,8 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
 #include 
 
 namespace clang {
@@ -128,13 +130,12 @@
 return true;
   }
 
-  bool VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc &TL) {
-// TemplateTypeParmTypeLoc does not have a TagDecl in its type ptr.
-addToken(TL.getBeginLoc(), TL.getDecl());
+  bool VisitTypedefTypeLoc(TypedefTypeLoc TL) {
+addType(TL.getBeginLoc(), TL.getTypePtr());
 return true;
   }
 
-  bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc &TL) {
+  bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) {
 if (const TemplateDecl *TD =
 TL.getTypePtr()->getTemplateName().getAsTemplateDecl())
   addToken(TL.getBeginLoc(), TD);
@@ -187,7 +188,10 @@
 if (TP->isBuiltinType())
   // Builtins must be special cased as they do not have a TagDecl.
   addToken(Loc, HighlightingKind::Primitive);
-if (const TagDecl *TD = TP->getAsTagDecl())
+if (auto *TD = dyn_cast(TP))
+  // TemplateTypeParmType also do not have a TagDecl.
+  addToken(Loc, TD->getDecl());
+if (auto *TD = TP->getAsTagDecl())
   addToken(Loc, TD);
   }
 


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
@@ -474,7 +474,7 @@
 $Macro[[assert]]($Variable[[x]] != $Function[[f]]());
   }
 )cpp",
-R"cpp(
+  R"cpp(
   struct $Class[[S]] {
 $Primitive[[float]] $Field[[Value]];
 $Class[[S]] *$Field[[Next]];
@@ -488,6 +488,21 @@
 // Highlights references to BindingDecls.
 $Variable[[B1]]++;
   }
+)cpp",
+  R"cpp(
+  template
+  class $Class[[A]] {
+using $TemplateParameter[[TemplateParam1]] = $TemplateParameter[[T]];
+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]]);
+  };
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(Test

[PATCH] D66516: [clangd] Highlight typedefs to template parameters as template parameters

2019-09-09 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:129
 if (const auto *TSI = TD->getTypeSourceInfo())
-  addType(TD->getLocation(), TSI->getTypeLoc().getTypePtr());
+  addType(TD->getLocation(), TSI->getTypeLoc().getType().getTypePtr());
 return true;

hokein wrote:
> nit: I think the previous ` TSI->getTypeLoc().getTypePtr()` should work?
Ah, good point, thanks! Fixed.
I've changed this code multiple types and didn't notice that the final result 
is just a no-op change.



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:270
+struct $Class[[A]] {
+  $Primitive[[void]] $Method[[foo]]($Class[[A]]*);
+};

hokein wrote:
> Is this change relevant to this patch?
A leftover from an older version of the patch. Removed, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66516



___
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

2019-09-09 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:780
+  // Prefer a mapping from the system symbols.
+  if (SystemSymbols) {
+if (auto Result = SystemSymbols->mapHeader(Header, QualifiedName))

hokein wrote:
> what's the interesting case where `SystemSymbols` is null?
> I think it should be fine to always use the standard symbol mapping here? 
> just use `getStandardSymbolMapping()->mapHeader()`, this would help to 
> simplify the CanonicalInclude interface (no `addSystemHeadersMapping` and 
> `SystemSymbols` members  are needed).
Fair question. I guess the only interesting case is unit tests - if we always 
have system include mapping, testing other things independently is impossible, 
therefore it's a bit more fragile.
But I don't think that's a real problem in practice.

Should we try to go even further, pass `LangOptions` to `CanonicalIncludes` on 
construction and always have a single defined mapping? I'll try to do that and 
update the patch accordingly. But let me know if you think that's a bad idea 
for some reason.


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

2019-09-09 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

I had the same reaction as Haojian to the original patch, thanks for clearing 
that up.

I do wonder whether we're microoptimizing for the tests too much, I don't think 
5% on the tests is worth very much in itself, unless it's speeding up real 
workloads or improving the code (it may well be).

I'm nervous about requiring langopts, it's convenient in tests/prototypes/etc 
to be able to default-construct CanonicalIncludes (e.g. a test that doesn't 
actually care about standard library mappings).


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

[clang-tools-extra] r371390 - [clangd] Update clangd-vscode docs to be more user-focused.

2019-09-09 Thread Sam McCall via cfe-commits

Author: sammccall
Date: Mon Sep  9 04:34:01 2019
New Revision: 371390

URL: http://llvm.org/viewvc/llvm-project?rev=371390&view=rev
Log:
[clangd] Update clangd-vscode docs to be more user-focused.

Summary: Relegate "updating the extension" docs to a separate file.

Reviewers: hokein, kadircet

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

Tags: #clang

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

Added:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/DEVELOPING.md
clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/

clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/complete.png   
(with props)

clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/diagnostics.png 
  (with props)
clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/extract.png 
  (with props)
clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/format.png  
 (with props)
clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/include.png 
  (with props)

clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/symbolsearch.png
   (with props)
clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/xrefs.png   
(with props)
Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/README.md

Added: clang-tools-extra/trunk/clangd/clients/clangd-vscode/DEVELOPING.md
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/DEVELOPING.md?rev=371390&view=auto
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/DEVELOPING.md (added)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/DEVELOPING.md Mon Sep  
9 04:34:01 2019
@@ -0,0 +1,53 @@
+# Development
+
+A guide of developing `vscode-clangd` extension.
+
+## Requirements
+
+* VS Code
+* node.js and npm
+
+## Steps
+
+1. Make sure you disable the installed `vscode-clangd` extension in VS Code.
+2. Make sure you have clangd in /usr/bin/clangd or edit src/extension.ts to
+point to the binary.
+3. In order to start a development instance of VS code extended with this, run:
+
+```bash
+   $ cd /path/to/clang-tools-extra/clangd/clients/clangd-vscode/
+   $ npm install
+   $ code .
+   # When VS Code starts, press .
+```
+
+# Contributing
+
+Please follow the exsiting code style when contributing to the extension, we
+recommend to run `npm run format` before sending a patch.
+
+# Publish to VS Code Marketplace
+
+New changes to `clangd-vscode` are not released until a new version is 
published
+to the marketplace.
+
+## Requirements
+
+* Make sure install the `vsce` command (`npm install -g vsce`)
+* `llvm-vs-code-extensions` account
+* Bump the version in `package.json`, and commit the change to upstream
+
+The extension is published under `llvm-vs-code-extensions` account, which is
+currently maintained by clangd developers. If you want to make a new release,
+please contact clangd-...@lists.llvm.org.
+
+## Steps
+
+```bash
+  $ cd /path/to/clang-tools-extra/clangd/clients/clangd-vscode/
+  # For the first time, you need to login in the account. vsce will ask you for
+the Personal Access Token, and remember it for future commands.
+  $ vsce login llvm-vs-code-extensions
+  $ vsce publish
+```
+

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/README.md
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/README.md?rev=371390&r1=371389&r2=371390&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/README.md (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/README.md Mon Sep  9 
04:34:01 2019
@@ -1,81 +1,103 @@
 # vscode-clangd
 
-Provides C/C++ language IDE features for VS Code using 
[clangd](https://clang.llvm.org/extra/clangd.html).
+Provides C/C++ language IDE features for VS Code using 
[clangd](https://clang.llvm.org/extra/clangd.html):
 
-## Usage
+ - code completion
+ - compile errors and warnings
+ - go-to-definition and cross references
+ - include management
+ - code formatting
+ - simple refactorings
 
-`vscode-clangd` provides the features designated by the [Language Server
-Protocol](https://github.com/Microsoft/language-server-protocol), such as
-code completion, code formatting and goto definition.
+## Setup
 
-**Note**: `clangd` is under heavy development, not all LSP features are
-implemented. See [Current 
Status](https://clang.llvm.org/extra/clangd/Features.html#complete-list-of-features)
-for details.
+### `clangd` server
 
-To use `vscode-clangd` extension in VS Code, you need to install 
`vscode-clangd`
-from VS Code extension marketplace.
+`clangd` is a language server that must be installed separately, see
+[https://clangd.github.io/installation.html](getting started).
+The vscode-clangd plugin will look for `clangd` o

[PATCH] D67092: [clangd] Update clangd-vscode docs to be more user-focused.

2019-09-09 Thread Sam McCall via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371390: [clangd] Update clangd-vscode docs to be more 
user-focused. (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67092?vs=218444&id=219323#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67092

Files:
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/DEVELOPING.md
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/README.md
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/complete.png
  
clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/diagnostics.png
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/extract.png
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/format.png
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/include.png
  
clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/symbolsearch.png
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/doc-assets/xrefs.png

Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/README.md
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/README.md
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/README.md
@@ -1,81 +1,103 @@
 # vscode-clangd
 
-Provides C/C++ language IDE features for VS Code using [clangd](https://clang.llvm.org/extra/clangd.html).
+Provides C/C++ language IDE features for VS Code using [clangd](https://clang.llvm.org/extra/clangd.html):
 
-## Usage
+ - code completion
+ - compile errors and warnings
+ - go-to-definition and cross references
+ - include management
+ - code formatting
+ - simple refactorings
 
-`vscode-clangd` provides the features designated by the [Language Server
-Protocol](https://github.com/Microsoft/language-server-protocol), such as
-code completion, code formatting and goto definition.
+## Setup
 
-**Note**: `clangd` is under heavy development, not all LSP features are
-implemented. See [Current Status](https://clang.llvm.org/extra/clangd/Features.html#complete-list-of-features)
-for details.
+### `clangd` server
 
-To use `vscode-clangd` extension in VS Code, you need to install `vscode-clangd`
-from VS Code extension marketplace.
+`clangd` is a language server that must be installed separately, see
+[https://clangd.github.io/installation.html](getting started).
+The vscode-clangd plugin will look for `clangd` on your PATH (you can change
+this in the settings).
 
-`vscode-clangd` will attempt to find the `clangd` binary on your `PATH`.
-Alternatively, the `clangd` executable can be specified in your VS Code
-`settings.json` file:
+### Project setup
 
-```json
-{
-"clangd.path": "/absolute/path/to/clangd"
-}
-```
+clangd is based on the clang C++ compiler, and understands even complex C++
+code.  However, you must tell clangd how your project is built (compile flags).
+[A `compile_commands.json` file](http://clang.llvm.org/docs/JSONCompilationDatabase.html)
+can usually be generated by your build system
+(e.g. by setting `-DCMAKE_EXPORT_COMPILE_COMMANDS=1` when building with CMake,
+or with
+[many other tools](https://sarcasm.github.io/notes/dev/compilation-database.html)).
 
-To obtain `clangd` binary, please see the [installing Clangd](https://clang.llvm.org/extra/clangd/Installation.html#installing-clangd).
+It should live at the top of your source tree: symlink or copy it there.
 
-## Development
+## Features
 
-A guide of developing `vscode-clangd` extension.
+### Code completion
 
-### Requirements
+Suggestions will appear as you type names, or after `.` or `->`.
+Because clangd uses a full C++ parser, code completion has access to precise
+type information.
 
-* VS Code
-* node.js and npm
+![Code completion](doc-assets/complete.png)
 
-### Steps
+### Errors, warnings, and clang-tidy
 
-1. Make sure you disable the installed `vscode-clangd` extension in VS Code.
-2. Make sure you have clangd in /usr/bin/clangd or edit src/extension.ts to
-point to the binary.
-3. In order to start a development instance of VS code extended with this, run:
+Code errors are shown as you type (both as red squiggle underlines, and in the
+"Problems" panel). These are the same as produced by the clang compiler, and
+suggested fixes can automatically be applied.
 
-```bash
-   $ cd /path/to/clang-tools-extra/clangd/clients/clangd-vscode/
-   $ npm install
-   $ code .
-   # When VS Code starts, press .
-```
+![Error with fix](doc-assets/diagnostics.png)
 
-## Contributing
+Most clang-tidy checks are supported (these can be enabled using a [.clang-tidy
+file](https://clang.llvm.org/extra/clang-tidy/)).
 
-Please follow the exsiting code style when contributing to the extension, we
-recommend to run `npm run format` before sending a patch.
+### Cross-reference

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

2019-09-09 Thread Sam McCall via Phabricator via cfe-commits

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



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

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



Comment at: clang-tools-extra/clangd/refactor/Tweak.cpp:97
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Failed to get absolute path for: " +
+ SM.getFileEntryForID(FID)->getName());

nit: "for edited file: "...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637



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

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

2019-09-09 Thread Rich Felker via Phabricator via cfe-commits

dalias added a comment.

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.


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

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

2019-09-09 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In D67172#1662888 , @sammccall wrote:

> I do wonder whether we're microoptimizing for the tests too much, I don't 
> think 5% on the tests is worth very much in itself, unless it's speeding up 
> real workloads or improving the code (it may well be).

Even though tests don't parse real C++ programs, I wouldn't call it a 
micro-optimization. Spending 5% on such a low-level operation is not ok, small 
things like that can add up and have a subtle effect on performance.
If there's an issue with making the code more complex, I'm happy to explore 
other ways to make it simpler. What would convince you that's a cleanup that 
improves things? What parts of the current version do you think are problematic?

> I'm nervous about requiring langopts, it's convenient in tests/prototypes/etc 
> to be able to default-construct CanonicalIncludes (e.g. a test that doesn't 
> actually care about standard library mappings).

We can have a factory method that constructs with default lang opts for tests, 
I don't think this would be an issue in the actual application code. In fact, I 
think it'll make the application code better.

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] D66637: [clangd] Support multifile edits as output of Tweaks

2019-09-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added inline comments.



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

sammccall wrote:
> kadircet wrote:
> > sammccall wrote:
> > > isn't this just
> > > `elog("Failed to format {0}: {1}", std::move(Err))`
> > IIUC, just printing the error doesn't actually mark it as checked, 
> > therefore causes an assertion failure (see `getPtr` vs `getPayload` in 
> > `llvm::Error`). And I couldn't see any special handling in `elog`, but not 
> > sure if `formatv object` has this.
> The special handling is on Logger.h:38. Do you see crashes?
ah OK, didn't notice the `fmt_consume` previously. no, I wasn't seeing any 
crashes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637



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

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

2019-09-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 219324.
kadircet marked 4 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637

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

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

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

2019-09-09 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a subscriber: hans.
MaskRay added a comment.

@hans Can this be put on your list of things for 9.0.1? I searched for `[meta] 
9.0.1` on bugs.llvm.org but can't find a release blocker bug so I have to 
bother you directly...


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

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

2019-09-09 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

In D65699#1662936 , @MaskRay wrote:

> @hans Can this be put on your list of things for 9.0.1? I searched for 
> `[meta] 9.0.1` on bugs.llvm.org but can't find a release blocker bug so I 
> have to bother you directly...


Right, the bug hasn't been filed yet. I've put it on my local list. Thanks!


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

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

2019-09-09 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

In D67172#1662916 , @ilya-biryukov 
wrote:

> In D67172#1662888 , @sammccall wrote:
>
> > I do wonder whether we're microoptimizing for the tests too much, I don't 
> > think 5% on the tests is worth very much in itself, unless it's speeding up 
> > real workloads or improving the code (it may well be).
>
>
> Even though tests don't parse real C++ programs, I wouldn't call it a 
> micro-optimization. Spending 5% on such a low-level operation is not ok, 
> small things like that can add up and have a subtle effect on performance.


Only if it's 5% of something meaningful. If the denominator isn't something we 
care about, then it's really "spending XXX usec is not ok" - which depends on 
XXX I think.

> If there's an issue with making the code more complex, I'm happy to explore 
> other ways to make it simpler. What would convince you that's a cleanup that 
> improves things? What parts of the current version do you think are 
> problematic?

You'll note that I didn't say that *anything* about the current version was 
problematic :-)

One way this could be simpler is if suffix mappings were gone, then the 
`StdIncludes` class would go away and we'd just be left with a 
`stringmap`. Then there'd be a performance win with almost no extra 
complexity.
The description says you're waiting for it to go away, but AFAIK nobody's 
working on this nor really knows how to eliminate it.

Some of the new complexity seems unneccesary even with this approach.
There's a lot of indirection around the initialization, an enum that gets 
passed around a bunch, constructors that switch over it.
I think this could be:

  StdIncludes *getStdIncludes(LangOpts) {
  if (is c++) {
 static pair<...> entries[] = ...;
 static StdIncludes *CPP = new StdIncludes(entries);
 // the things that apply everywhere are in the StdIncludes constructor
 return CPP;
  } else if (is c) {
 ...
  } else {
static StdIncludes *Others = new StdIncludes({});
return Others;
  }
  }



>> I'm nervous about requiring langopts, it's convenient in 
>> tests/prototypes/etc to be able to default-construct CanonicalIncludes (e.g. 
>> a test that doesn't actually care about standard library mappings).
> 
> We can have a factory method that constructs with default lang opts for 
> tests, I don't think this would be an issue in the actual application code. 
> In fact, I think it'll make the application code better.

I'm not saying that there's no way to work around it, or that it's fatal, but 
my opinion is that this would be worse than the current state.




Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:22
 
-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;
+enum class StdIncludes::SymbolsFlavour { Cpp, C11, Other };
+namespace {

Why C11 and not C?


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] D67264: [clangd] Collect location of macro definition in the ParsedAST

2019-09-09 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.

Comment at: clang-tools-extra/clangd/ParsedAST.cpp:104
 // *definitions* in the preamble region of the main file).
 class CollectMainFileMacroExpansions : public PPCallbacks {
   const SourceManager &SM;

hokein wrote:
> The name doesn't fit into what it does anymore, maybe rename to 
> `CollectMainFileMacroLocations`?
Agree, maybe even shorter - `CollectMainFileMacros`?

Comment at: clang-tools-extra/clangd/ParsedAST.h:125
+  /// preamble.
   /// Does not include expansions from inside other macro expansions.
   std::vector MainFileMacroExpLocs;

NIT: s/Does not include expansions/Does not include **locations**?

To avoid confusion, since they are **definitions and expansions** now.

Comment at: clang-tools-extra/clangd/ParsedAST.h:126
   /// Does not include expansions from inside other macro expansions.
   std::vector MainFileMacroExpLocs;
   // Data, stored after parsing.

Could we you rename this to `MacroIdentifierLocs`? Or something similar not 
mentioning expansions in the name.

`MacroExpLocs` could still be interpreted as locations of expansions, rather 
than locations of all macro identifiers.

Comment at: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp:234
+  namespace {
+#define ^MACRO_ARGS(X, Y) X Y
 ^ID(int A);

We should probably keep the test as is and make sure we also collect inside the 
preamble.
It's ok to leave a FIXME and do this in a separate change, but let's not change 
the test to hide the fact that we are not highlighting macros inside the 
preamble anymore.

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

[clang-tools-extra] r371392 - [clangd] Support multifile edits as output of Tweaks

2019-09-09 Thread Kadir Cetinkaya via cfe-commits

Author: kadircet
Date: Mon Sep  9 05:28:44 2019
New Revision: 371392

URL: http://llvm.org/viewvc/llvm-project?rev=371392&view=rev
Log:
[clangd] Support multifile edits as output of Tweaks

Summary:
First patch for propogating multifile changes from tweak outputs to LSP
WorkspaceEdits.

Uses SM to convert tooling::Replacements to TextEdits.
Errors out if there are any inconsistencies between the draft version and the
version generated the edits.

Reviewers: sammccall

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

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=371392&r1=371391&r2=371392&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Mon Sep  9 05:28:44 2019
@@ -8,6 +8,7 @@
 
 #include "ClangdLSPServer.h"
 #include "Diagnostics.h"
+#include "DraftStore.h"
 #include "FormattedString.h"
 #include "GlobalCompilationDatabase.h"
 #include "Protocol.h"
@@ -20,11 +21,15 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/SHA1.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -93,6 +98,35 @@ std::vector> bu
   return LookupTable;
 }
 
+// Makes sure edits in \p E are applicable to latest file contents reported by
+// editor. If not generates an error message containing information about files
+// that needs to be saved.
+llvm::Error validateEdits(const DraftStore &DraftMgr, const Tweak::Effect &E) {
+  size_t InvalidFileCount = 0;
+  llvm::StringRef LastInvalidFile;
+  for (const auto &It : E.ApplyEdits) {
+if (auto Draft = DraftMgr.getDraft(It.first())) {
+  // If the file is open in user's editor, make sure the version we
+  // saw and current version are compatible as this is the text that
+  // will be replaced by editors.
+  if (!It.second.canApplyTo(*Draft)) {
+++InvalidFileCount;
+LastInvalidFile = It.first();
+  }
+}
+  }
+  if (!InvalidFileCount)
+return llvm::Error::success();
+  if (InvalidFileCount == 1)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "File must be saved first: " +
+   LastInvalidFile);
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "Files must be saved first: " + LastInvalidFile + " (and " +
+  llvm::to_string(InvalidFileCount - 1) + " others)");
+}
+
 } // namespace
 
 // MessageHandler dispatches incoming LSP messages.
@@ -627,7 +661,8 @@ void ClangdLSPServer::onCommand(const Ex
   if (!R)
 return Reply(R.takeError());
 
-  assert(R->ShowMessage || (R->ApplyEdit && "tweak has no effect"));
+  assert(R->ShowMessage ||
+ (!R->ApplyEdits.empty() && "tweak has no effect"));
 
   if (R->ShowMessage) {
 ShowMessageParams Msg;
@@ -635,15 +670,21 @@ void ClangdLSPServer::onCommand(const Ex
 Msg.type = MessageType::Info;
 notify("window/showMessage", Msg);
   }
-  if (R->ApplyEdit) {
-WorkspaceEdit WE;
-WE.changes.emplace();
-(*WE.changes)[File.uri()] = replacementsToEdits(Code, *R->ApplyEdit);
-// ApplyEdit will take care of calling Reply().
-return ApplyEdit(std::move(WE), "Tweak applied.", std::move(Reply));
-  }
   // When no edit is specified, make sure we Reply().
-  return Reply("Tweak applied.");
+  if (R->ApplyEdits.empty())
+return Reply("Tweak applied.");
+
+  if (auto Err = validateEdits(

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

2019-09-09 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In D67172#1662945 , @sammccall wrote:

> Only if it's 5% of something meaningful. If the denominator isn't something 
> we care about, then it's really "spending XXX usec is not ok" - which depends 
> on XXX I think.

Agree, but unit-test time is meaningful clangd developers. Not saying this is 
more important than real use-cases, but keeping the unit tests enables us to 
iterate faster.

> One way this could be simpler is if suffix mappings were gone, then the 
> `StdIncludes` class would go away and we'd just be left with a 
> `stringmap`. Then there'd be a performance win with almost no extra 
> complexity.
>  The description says you're waiting for it to go away, but AFAIK nobody's 
> working on this nor really knows how to eliminate it.

We seem to know how to eliminate this in principle. See @hokein's comment, we 
could try parsing man pages or even the headers themselves and building a 
corresponding symbol map.
But this patch does not add suffix mapping, so I don't think that's relevant - 
this patch tries to avoid redundant initialization; not remove suffix maps.

> Some of the new complexity seems unneccesary even with this approach.
>  There's a lot of indirection around the initialization, an enum that gets 
> passed around a bunch, constructors that switch over it.
>  I think this could be:

Will update the patch. Although I find the current approach more direct, I will 
happily change the code according to your suggestion.

>>> I'm nervous about requiring langopts, it's convenient in 
>>> tests/prototypes/etc to be able to default-construct CanonicalIncludes 
>>> (e.g. a test that doesn't actually care about standard library mappings).
>> 
>> We can have a factory method that constructs with default lang opts for 
>> tests, I don't think this would be an issue in the actual application code. 
>> In fact, I think it'll make the application code better.
> 
> I'm not saying that there's no way to work around it, or that it's fatal, but 
> my opinion is that this would be worse than the current state.

If we care about having a default constructor, we can default to using 
`StdIncludes` with a default mapping. I'll avoid requiring `LangOptions` in the 
constructor in that case. Thanks for the early feedback!

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] D66637: [clangd] Support multifile edits as output of Tweaks

2019-09-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371392: [clangd] Support multifile edits as output of Tweaks 
(authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66637?vs=219324&id=219325#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66637

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

Index: clang-tools-extra/trunk/clangd/SourceCode.cpp
===
--- clang-tools-extra/trunk/clangd/SourceCode.cpp
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp
@@ -11,6 +11,7 @@
 #include "FuzzyMatch.h"
 #include "Logger.h"
 #include "Protocol.h"
+#include "refactor/Tweak.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
@@ -19,14 +20,21 @@
 #include "clang/Format/Format.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/LineIterator.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/SHA1.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/xxhash.h"
 #include 
 
@@ -562,7 +570,7 @@
   }
 
   // Handle the symbolic link path case where the current working directory
-  // (getCurrentWorkingDirectory) is a symlink./ We always want to the real
+  // (getCurrentWorkingDirectory) is a symlink. We always want to the real
   // file path (instead of the symlink path) for the  C++ symbols.
   //
   // Consider the following example:
@@ -908,5 +916,57 @@
   return None;
 }
 
+llvm::Expected Edit::apply() const {
+  return tooling::applyAllReplacements(InitialCode, Replacements);
+}
+
+std::vector Edit::asTextEdits() const {
+  return replacementsToEdits(InitialCode, Replacements);
+}
+
+bool Edit::canApplyTo(llvm::StringRef Code) const {
+  // Create line iterators, since line numbers are important while applying our
+  // edit we cannot skip blank lines.
+  auto LHS = llvm::MemoryBuffer::getMemBuffer(Code);
+  llvm::line_iterator LHSIt(*LHS, /*SkipBlanks=*/false);
+
+  auto RHS = llvm::MemoryBuffer::getMemBuffer(InitialCode);
+  llvm::line_iterator RHSIt(*RHS, /*SkipBlanks=*/false);
+
+  // Compare the InitialCode we prepared the edit for with the Code we received
+  // line by line to make sure there are no differences.
+  // FIXME: This check is too conservative now, it should be enough to only
+  // check lines around the replacements contained inside the Edit.
+  while (!LHSIt.is_at_eof() && !RHSIt.is_at_eof()) {
+if (*LHSIt != *RHSIt)
+  return false;
+++LHSIt;
+++RHSIt;
+  }
+
+  // After we reach EOF for any of the files we make sure the other one doesn't
+  // contain any additional content except empty lines, they should not
+  // interfere with the edit we produced.
+  while (!LHSIt.is_at_eof()) {
+if (!LHSIt->empty())
+  return false;
+++LHSIt;
+  }
+  while (!RHSIt.is_at_eof()) {
+if (!RHSIt->empty())
+  return false;
+++RHSIt;
+  }
+  return true;
+}
+
+llvm::Error reformatEdit(Edit &E, const format::FormatStyle &Style) {
+  if (auto NewEdits = cleanupAndFormat(E.InitialCode, E.Replacements, Style))
+E.Replacements = std::move(*NewEdits);
+  else
+return NewEdits.takeError();
+  return llvm::Error::success();
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp
@@ -12,6 +12,7 @@
 #include "Format.h"
 #inc

[clang-tools-extra] r371394 - [clang-doc] sys::fs::F_None -> OF_None. NFC

2019-09-09 Thread Fangrui Song via cfe-commits

Author: maskray
Date: Mon Sep  9 05:42:10 2019
New Revision: 371394

URL: http://llvm.org/viewvc/llvm-project?rev=371394&view=rev
Log:
[clang-doc] sys::fs::F_None -> OF_None. NFC

F_None, F_Text and F_Append are kept for compatibility.

Modified:
clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp

Modified: clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp?rev=371394&r1=371393&r2=371394&view=diff
==
--- clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp Mon Sep  9 05:42:10 2019
@@ -889,7 +889,7 @@ static llvm::Error SerializeIndex(ClangD
   llvm::SmallString<128> FilePath;
   llvm::sys::path::native(CDCtx.OutDirectory, FilePath);
   llvm::sys::path::append(FilePath, "index_json.js");
-  llvm::raw_fd_ostream OS(FilePath, FileErr, llvm::sys::fs::F_None);
+  llvm::raw_fd_ostream OS(FilePath, FileErr, llvm::sys::fs::OF_None);
   if (FileErr != OK) {
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"error creating index file: " +
@@ -938,7 +938,7 @@ static llvm::Error GenIndex(const ClangD
   llvm::SmallString<128> IndexPath;
   llvm::sys::path::native(CDCtx.OutDirectory, IndexPath);
   llvm::sys::path::append(IndexPath, "index.html");
-  llvm::raw_fd_ostream IndexOS(IndexPath, FileErr, llvm::sys::fs::F_None);
+  llvm::raw_fd_ostream IndexOS(IndexPath, FileErr, llvm::sys::fs::OF_None);
   if (FileErr != OK) {
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"error creating main index: " +

Modified: clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp?rev=371394&r1=371393&r2=371394&view=diff
==
--- clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp Mon Sep  9 05:42:10 
2019
@@ -303,7 +303,7 @@ int main(int argc, const char **argv) {
   }
   std::error_code FileErr;
   llvm::raw_fd_ostream InfoOS(InfoPath.get(), FileErr,
-  llvm::sys::fs::F_None);
+  llvm::sys::fs::OF_None);
   if (FileErr != OK) {
 llvm::errs() << "Error opening info file: " << FileErr.message()
  << "\n";


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

[clang-tools-extra] r371396 - [clang-tidy] Fix bug in bugprone-use-after-move check

2019-09-09 Thread Yitzhak Mandelbaum via cfe-commits

Author: ymandel
Date: Mon Sep  9 05:59:14 2019
New Revision: 371396

URL: http://llvm.org/viewvc/llvm-project?rev=371396&view=rev
Log:
[clang-tidy] Fix bug in bugprone-use-after-move check

Summary:
The bugprone-use-after-move check exhibits false positives for certain uses of
the C++17 if/switch init statements. These false positives are caused by a bug
in the ExprSequence calculations.

This revision adds tests for the false positives and fixes the corresponding
sequence calculation.

Reviewers: gribozavr

Subscribers: xazax.hun, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp

Modified: clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp?rev=371396&r1=371395&r2=371396&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp Mon Sep  9 
05:59:14 2019
@@ -145,17 +145,24 @@ const Stmt *ExprSequence::getSequenceSuc
 return ForRange->getBody();
 } else if (const auto *TheIfStmt = dyn_cast(Parent)) {
   // If statement:
-  // - Sequence init statement before variable declaration.
+  // - Sequence init statement before variable declaration, if present;
+  //   before condition evaluation, otherwise.
   // - Sequence variable declaration (along with the expression used to
   //   initialize it) before the evaluation of the condition.
-  if (S == TheIfStmt->getInit())
-return TheIfStmt->getConditionVariableDeclStmt();
+  if (S == TheIfStmt->getInit()) {
+if (TheIfStmt->getConditionVariableDeclStmt() != nullptr)
+  return TheIfStmt->getConditionVariableDeclStmt();
+return TheIfStmt->getCond();
+  }
   if (S == TheIfStmt->getConditionVariableDeclStmt())
 return TheIfStmt->getCond();
 } else if (const auto *TheSwitchStmt = dyn_cast(Parent)) {
   // Ditto for switch statements.
-  if (S == TheSwitchStmt->getInit())
-return TheSwitchStmt->getConditionVariableDeclStmt();
+  if (S == TheSwitchStmt->getInit()) {
+if (TheSwitchStmt->getConditionVariableDeclStmt() != nullptr)
+  return TheSwitchStmt->getConditionVariableDeclStmt();
+return TheSwitchStmt->getCond();
+  }
   if (S == TheSwitchStmt->getConditionVariableDeclStmt())
 return TheSwitchStmt->getCond();
 } else if (const auto *TheWhileStmt = dyn_cast(Parent)) {

Modified: clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp?rev=371396&r1=371395&r2=371396&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp Mon Sep 
 9 05:59:14 2019
@@ -1182,7 +1182,10 @@ void forRangeSequences() {
 
 // If a variable is declared in an if, while or switch statement, the init
 // statement (for if and switch) is sequenced before the variable declaration,
-// which in turn is sequenced before the evaluation of the condition.
+// which in turn is sequenced before the evaluation of the condition. We place
+// all tests inside a for loop to ensure that the checker understands the
+// sequencing. If it didn't, then the loop would trigger the "moved twice"
+// logic.
 void ifWhileAndSwitchSequenceInitDeclAndCondition() {
   for (int i = 0; i < 10; ++i) {
 A a1;
@@ -1192,15 +1195,44 @@ void ifWhileAndSwitchSequenceInitDeclAnd
   }
   for (int i = 0; i < 10; ++i) {
 A a1;
+if (A a2 = std::move(a1); a2) {
+  std::move(a2);
+}
+  }
+  for (int i = 0; i < 10; ++i) {
+A a1;
 if (A a2 = std::move(a1); A a3 = std::move(a2)) {
   std::move(a3);
 }
   }
+  for (int i = 0; i < 10; ++i) {
+// init followed by condition with move, but without variable declaration.
+if (A a1; A(std::move(a1)).getInt() > 0) {}
+  }
+  for (int i = 0; i < 10; ++i) {
+if (A a1; A(std::move(a1)).getInt() > a1.getInt()) {}
+// CHECK-NOTES: [[@LINE-1]]:43: warning: 'a1' used after it was moved
+// CHECK-NOTES: [[@LINE-2]]:15: note: move occurred here
+// CHECK-NOTES: [[@LINE-3]]:43: note: the use and move are unsequenced
+  }
+  for (int i = 0; i < 10; ++i) {
+A a1;
+if (A a2 = std::move(a1); A(a1) > 0) {}
+// CHECK-NOTES: [[@LINE-1]]:33: warning: 'a1' used after it was moved
+// CHECK-NOTES: [[@LINE-2]]:16: note: move occurred here
+  }
   while (A a = A()) {
 std::move(a);
   }
   for (int i = 0; i < 10; ++i) {
 A a1;
+switch

[PATCH] D67292: [clang-tidy] Fix bug in bugprone-use-after-move check

2019-09-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
ymandel marked an inline comment as done.
Closed by commit rL371396: [clang-tidy] Fix bug in bugprone-use-after-move 
check (authored by ymandel, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67292?vs=219152&id=219334#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67292

Files:
  clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
  clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp


Index: clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
@@ -145,17 +145,24 @@
 return ForRange->getBody();
 } else if (const auto *TheIfStmt = dyn_cast(Parent)) {
   // If statement:
-  // - Sequence init statement before variable declaration.
+  // - Sequence init statement before variable declaration, if present;
+  //   before condition evaluation, otherwise.
   // - Sequence variable declaration (along with the expression used to
   //   initialize it) before the evaluation of the condition.
-  if (S == TheIfStmt->getInit())
-return TheIfStmt->getConditionVariableDeclStmt();
+  if (S == TheIfStmt->getInit()) {
+if (TheIfStmt->getConditionVariableDeclStmt() != nullptr)
+  return TheIfStmt->getConditionVariableDeclStmt();
+return TheIfStmt->getCond();
+  }
   if (S == TheIfStmt->getConditionVariableDeclStmt())
 return TheIfStmt->getCond();
 } else if (const auto *TheSwitchStmt = dyn_cast(Parent)) {
   // Ditto for switch statements.
-  if (S == TheSwitchStmt->getInit())
-return TheSwitchStmt->getConditionVariableDeclStmt();
+  if (S == TheSwitchStmt->getInit()) {
+if (TheSwitchStmt->getConditionVariableDeclStmt() != nullptr)
+  return TheSwitchStmt->getConditionVariableDeclStmt();
+return TheSwitchStmt->getCond();
+  }
   if (S == TheSwitchStmt->getConditionVariableDeclStmt())
 return TheSwitchStmt->getCond();
 } else if (const auto *TheWhileStmt = dyn_cast(Parent)) {
Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
@@ -1182,7 +1182,10 @@
 
 // If a variable is declared in an if, while or switch statement, the init
 // statement (for if and switch) is sequenced before the variable declaration,
-// which in turn is sequenced before the evaluation of the condition.
+// which in turn is sequenced before the evaluation of the condition. We place
+// all tests inside a for loop to ensure that the checker understands the
+// sequencing. If it didn't, then the loop would trigger the "moved twice"
+// logic.
 void ifWhileAndSwitchSequenceInitDeclAndCondition() {
   for (int i = 0; i < 10; ++i) {
 A a1;
@@ -1192,15 +1195,44 @@
   }
   for (int i = 0; i < 10; ++i) {
 A a1;
+if (A a2 = std::move(a1); a2) {
+  std::move(a2);
+}
+  }
+  for (int i = 0; i < 10; ++i) {
+A a1;
 if (A a2 = std::move(a1); A a3 = std::move(a2)) {
   std::move(a3);
 }
   }
+  for (int i = 0; i < 10; ++i) {
+// init followed by condition with move, but without variable declaration.
+if (A a1; A(std::move(a1)).getInt() > 0) {}
+  }
+  for (int i = 0; i < 10; ++i) {
+if (A a1; A(std::move(a1)).getInt() > a1.getInt()) {}
+// CHECK-NOTES: [[@LINE-1]]:43: warning: 'a1' used after it was moved
+// CHECK-NOTES: [[@LINE-2]]:15: note: move occurred here
+// CHECK-NOTES: [[@LINE-3]]:43: note: the use and move are unsequenced
+  }
+  for (int i = 0; i < 10; ++i) {
+A a1;
+if (A a2 = std::move(a1); A(a1) > 0) {}
+// CHECK-NOTES: [[@LINE-1]]:33: warning: 'a1' used after it was moved
+// CHECK-NOTES: [[@LINE-2]]:16: note: move occurred here
+  }
   while (A a = A()) {
 std::move(a);
   }
   for (int i = 0; i < 10; ++i) {
 A a1;
+switch (A a2 = std::move(a1); a2) {
+  case true:
+std::move(a2);
+}
+  }
+  for (int i = 0; i < 10; ++i) {
+A a1;
 switch (A a2 = a1; A a3 = std::move(a2)) {
   case true:
 std::move(a3);


Index: clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
@@ -145,17 +145,24 @@
 return ForRange->getBody();
 } else if (const auto *TheIfStmt = dyn_cast(Parent)) {
   // If statemen

[PATCH] D67292: [clang-tidy] Fix bug in bugprone-use-after-move check

2019-09-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

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



Comment at: clang-tools-extra/test/clang-tidy/bugprone-use-after-move.cpp:1198
+}
+  }
   for (int i = 0; i < 10; ++i) {

mboehme wrote:
> gribozavr wrote:
> > ymandel wrote:
> > > gribozavr wrote:
> > > > Unless you think it is redundant, could you also add
> > > > 
> > > > ```
> > > > if (A a1; A(std::move(a2)).getInt() > 0) {}
> > > > ```
> > > > 
> > > > Also some true positive tests would be good:
> > > > 
> > > > ```
> > > > if (A a1; A(std::move(a2)).getInt() > A(std::move(a2)).getInt()) {}
> > > > ```
> > > > 
> > > > ```
> > > > A a1;
> > > > if (A a2 = std::move(a1); A(std::move(a1)) > 0) {}
> > > > ```
> > > Done, but any idea why everything in this function is placed inside a 
> > > loop?  Looks like its just for scoping, but then why not just a compound 
> > > statement, as is done above? This feels very odd.
> > I think it is to ensure that the checker understands the sequencing. If it 
> > didn't, then the loop would trigger the "moved twice" logic.
> (Original author of the test here.)
> 
> Correct. I should probably have added a comment explaining this when I wrote 
> the test. Feel free to add such a comment.
Thanks for clarifying. I added a sentence to the comment as you suggested.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67292



___
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

2019-09-09 Thread Ilya Biryukov via Phabricator via cfe-commits

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

- Alternative approach - do not add extra classes, aim to minimize changes


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(&Includes, Language);
+  Includes.addSystemHeadersMapping(Language);
   CollectorOpts.Includes = &Includes;
   runSymbolCollector("namespace std { class string {}; }", /*Main=*/"");
   EXPECT_THAT(Symbols,
Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -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(&CI, 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(&CI, 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 &CI, llvm::StringRef InFile) override {
 CI.getPreprocessor().addCommentHandler(PragmaHandler.get());
-addSystemHeadersMapping(Includes.get(), CI.getLangOpts());
+Includes->addSystemHeadersMapping(CI.getLangOpts());
 if (IncludeGraphCallback != nullptr)
   CI.getPreprocessor().addPPCallbacks(
   std::make_unique(CI.getSourceManager(), IG));
Index: clang-tools-extra/clangd/index/CanonicalIncludes.h
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.h
+++ clang-tools-extra/clangd/index/CanonicalIncludes.h
@@ -34,37 +34,37 @@
 /// Only const methods (i.e. mapHeader) in this class are thread safe.
 class CanonicalIncludes {
 public:
-  CanonicalIncludes() = default;
-
   /// Adds a string-to-string mapping from \p Path to \p CanonicalPath.
   void addMapping(llvm::StringRef Path, llvm::StringR

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

2019-09-09 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

The new version looks very much like the older one, with a few additions to not 
re-initialize data twice.
PTAL and let me know what you think.


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

2019-09-09 Thread Ilya Biryukov via Phabricator via cfe-commits

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



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:748
+llvm::StringRef CanonicalPath = Pair.second;
+int Components = std::distance(llvm::sys::path::begin(Suffix),
+   llvm::sys::path::end(Suffix));

Note that `addSuffixMapping`/`findSuffixMapping` are not paired anymore, which 
I personally find to be odd.
But let me know whether that's a concern someone else shares.


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] D67304: Unify checking enumerator values in ObjC, C, and MSVC modes

2019-09-09 Thread Hans Wennborg via Phabricator via cfe-commits

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

lgtm with comment

> The main functional change is that -fno-ms-compatibility now no longer
>  sends us down the hard error diagnostic code path for ObjC fixed enums.
>  Instead, complete-but-not-fixed MS enums go down the C99 codepath which
>  issues a -Wmicrosoft-enum-value warning about the enum value not being
>  representable as an 'int'. This was highlighted as the single largest
>  blocker to compiling windows.h with -fno-ms-compatibility, so this
>  should make that feasible.

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


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] D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test

2019-09-09 Thread Tom Stellard via Phabricator via cfe-commits

tstellar added inline comments.



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

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.


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] D67264: [clangd] Collect location of macro definition in the ParsedAST

2019-09-09 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 219343.
hokein marked 4 inline comments as done.
hokein added a comment.

Rename all things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67264

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  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
@@ -392,18 +392,19 @@
   R"cpp(
   #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
   #define DEF_CLASS(T) class T {};
+  // Preamble ends.
   $Macro[[DEF_MULTIPLE]](XYZ);
   $Macro[[DEF_MULTIPLE]](XYZW);
   $Macro[[DEF_CLASS]]($Class[[A]])
-  #define MACRO_CONCAT(X, V, T) T foo##X = V
-  #define DEF_VAR(X, V) int X = V
-  #define DEF_VAR_T(T, X, V) T X = V
-  #define DEF_VAR_REV(V, X) DEF_VAR(X, V)
-  #define CPY(X) X
-  #define DEF_VAR_TYPE(X, Y) X Y
-  #define SOME_NAME variable
-  #define SOME_NAME_SET variable2 = 123
-  #define INC_VAR(X) X += 2
+  #define $Macro[[MACRO_CONCAT]](X, V, T) T foo##X = V
+  #define $Macro[[DEF_VAR]](X, V) int X = V
+  #define $Macro[[DEF_VAR_T]](T, X, V) T X = V
+  #define $Macro[[DEF_VAR_REV]](V, X) DEF_VAR(X, V)
+  #define $Macro[[CPY]](X) X
+  #define $Macro[[DEF_VAR_TYPE]](X, Y) X Y
+  #define $Macro[[SOME_NAME]] variable
+  #define $Macro[[SOME_NAME_SET]] variable2 = 123
+  #define $Macro[[INC_VAR]](X) X += 2
   $Primitive[[void]] $Function[[foo]]() {
 $Macro[[DEF_VAR]]($LocalVariable[[X]],  123);
 $Macro[[DEF_VAR_REV]](908, $LocalVariable[[XY]]);
@@ -422,8 +423,8 @@
   $Macro[[DEF_VAR]]($Variable[[XYZ]], 567);
   $Macro[[DEF_VAR_REV]](756, $Variable[[AB]]);
 
-  #define CALL_FN(F) F();
-  #define DEF_FN(F) void F ()
+  #define $Macro[[CALL_FN]](F) F();
+  #define $Macro[[DEF_FN]](F) void F ()
   $Macro[[DEF_FN]]($Function[[g]]) {
 $Macro[[CALL_FN]]($Function[[foo]]);
   }
@@ -431,6 +432,7 @@
   R"cpp(
   #define fail(expr) expr
   #define assert(COND) if (!(COND)) { fail("assertion failed" #COND); }
+  // Preamble ends.
   $Primitive[[int]] $Variable[[x]];
   $Primitive[[int]] $Variable[[y]];
   $Primitive[[int]] $Function[[f]]();
@@ -439,7 +441,7 @@
 $Macro[[assert]]($Variable[[x]] != $Function[[f]]());
   }
 )cpp",
-R"cpp(
+  R"cpp(
   struct $Class[[S]] {
 $Primitive[[float]] $Field[[Value]];
 $Class[[S]] *$Field[[Next]];
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -230,26 +230,28 @@
 TEST(ParsedASTTest, CollectsMainFileMacroExpansions) {
   Annotations TestCase(R"cpp(
 #define MACRO_ARGS(X, Y) X Y
+// - premable ends, macros inside preamble are not considered in main file.
 ^ID(int A);
 // Macro arguments included.
 ^MACRO_ARGS(^MACRO_ARGS(^MACRO_EXP(int), A), ^ID(= 2));
 
 // Macro names inside other macros not included.
-#define FOO BAR
-#define BAR 1
+#define ^MACRO_ARGS2(X, Y) X Y
+#define ^FOO BAR
+#define ^BAR 1
 int A = ^FOO;
 
 // Macros from token concatenations not included.
-#define CONCAT(X) X##A()
-#define PREPEND(X) MACRO##X()
-#define MACROA() 123
+#define ^CONCAT(X) X##A()
+#define ^PREPEND(X) MACRO##X()
+#define ^MACROA() 123
 int B = ^CONCAT(MACRO);
 int D = ^PREPEND(A)
 
 // Macros included not from preamble not included.
 #include "foo.inc"
 
-#define assert(COND) if (!(COND)) { printf("%s", #COND); exit(0); }
+#define ^assert(COND) if (!(COND)) { printf("%s", #COND); exit(0); }
 
 void test() {
   // Includes macro expansions in arguments that are expressions
@@ -268,8 +270,7 @@
 int D = DEF;
   )cpp";
   ParsedAST AST = TU.build();
-  const std::vector &MacroExpansionLocations =
-  AST.getMainFileExpansions();
+  const std::vector &MacroExpansionLocations = AST.getMacros();
   std::vector MacroExpansionPositions;
   for (const auto &L : MacroExpansionLocations)
 MacroExpansionPositions.push_back(
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -35,9 +35,7 @@
 TraverseAST(AST.getASTContext());
 //

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

2019-09-09 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

Thanks for the suggestions on the new names.


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] D67290: [clangd] Add a new highlighting kind for typedefs

2019-09-09 Thread Haojian Wu via Phabricator via cfe-commits

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



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

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



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:501
 
 // These typedefs are not yet highlighted, their types are complicated.
+using $Typedef[[Pointer]] = $TemplateParameter[[T]] *;

nit: the comment is out-of-date with this patch.


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] D67172: [clangd] Use pre-populated mappings for standard symbols

2019-09-09 Thread Sam McCall via Phabricator via cfe-commits

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

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

In D67172#1662957 , @ilya-biryukov 
wrote:

> In D67172#1662945 , @sammccall wrote:
>
> > Only if it's 5% of something meaningful. If the denominator isn't something 
> > we care about, then it's really "spending XXX usec is not ok" - which 
> > depends on XXX I think.
>
>
> Agree, but unit-test time is meaningful clangd developers. Not saying this is 
> more important than real use-cases, but keeping the unit tests enables us to 
> iterate faster.

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.

>> One way this could be simpler is if suffix mappings were gone, then the 
>> `StdIncludes` class would go away and we'd just be left with a 
>> `stringmap`. Then there'd be a performance win with almost no extra 
>> complexity.
>>  The description says you're waiting for it to go away, but AFAIK nobody's 
>> working on this nor really knows how to eliminate it.
> 
> We seem to know how to eliminate this in principle. See @hokein's comment, we 
> could try parsing man pages or even the headers themselves and building a 
> corresponding symbol map.

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

> But this patch does not add suffix mapping, so I don't think that's relevant 
> - this patch tries to avoid redundant initialization; not remove suffix maps.

This patch adds code shoveling around suffix maps, and so increases the cost of 
this feature. (Less so after latest changes). It also increases the scope of a 
future patch that would remove suffix maps. Sequencing this the other way would 
be better, if removing suffix mapping was straightforward. (I don't think it is)

>> Some of the new complexity seems unneccesary even with this approach.
>>  There's a lot of indirection around the initialization, an enum that gets 
>> passed around a bunch, constructors that switch over it.
>>  I think this could be:
> 
> Will update the patch. Although I find the current approach more direct, I 
> will happily change the code according to your suggestion.

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

Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:41

+  if (!SuffixHeaderMapping)
+return Header;

nit: can we write `if (SuffixHeaderMapping) { ... }` for consistency with above?

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

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?

Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:91
   //   - symbols with multiple headers (e.g. std::move)
   static constexpr std::pair SystemHeaderMap[] = {
   {"include/__stddef_max_align_t.h", ""},

I think this could just be
```
static const auto *SystemHeaderMap = new llvm::StringMap{
...
}
```
skipping the intermediate array, and doing the initialize-once here
(if we can drop the struct)

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

if you want to save CPU, move to the scope where it's used? Lit tests and many 
interesting subsets will never use C.

Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.h:60
   /// A map from a suffix (one or components of a path) to a canonical path.
-  llvm::StringMap SuffixHeaderMapping;
+  /// Used only for mapping standard headers.
+  const llvm::StringMap *SuffixHeaderMapping = nullptr;

nit: maybe StdSuffixHeaderMapping to clarify the purpose, now these are std-only

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] D67341: [clangd] Simplify semantic highlighting visitor

2019-09-09 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

Thanks!

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


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] D65699: [Driver] Prioritize SYSROOT/usr/include over RESOURCE_DIR/include on linux-musl

2019-09-09 Thread 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

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

2019-09-09 Thread 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 &TestCase : 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(Source

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

2019-09-09 Thread 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

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

2019-09-09 Thread 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&view=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&r1=371401&r2=371402&view=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 &operator<<(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&r1=371401&r2=371402&view=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&r1=371401&r2=371402&view=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:  ],
 # C

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

2019-09-09 Thread 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&id=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 &TestCase : TestCases) {
Index: clang-

r371403 - Merge note_ovl_builtin_candidate diagnostics; NFC

2019-09-09 Thread 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&view=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&r1=371402&r2=371403&view=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&r1=371402&r2=371403&view=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

2019-09-09 Thread 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

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

2019-09-09 Thread 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] D67172: [clangd] Use pre-populated mappings for standard symbols

2019-09-09 Thread 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] D67031: [Clang][Bundler] Error reporting improvements

2019-09-09 Thread 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] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

2019-09-09 Thread 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] D67172: [clangd] Use pre-populated mappings for standard symbols

2019-09-09 Thread 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] D67172: [clangd] Use pre-populated mappings for standard symbols

2019-09-09 Thread 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(&Includes, Language);
+  Includes.addSystemHeadersMapping(Language);
   CollectorOpts.Includes = &Includes;
   runSymbolCollector("namespace std { class string {}; }", /*Main=*/"");
   EXPECT_THAT(Symbols,
Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -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(&CI, 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(&CI, 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 &CI, llvm::StringRef InFile) override {
 CI.getPreprocessor().addCommentHandler(PragmaHandler.get());
-addSystemHeadersMapping(Includes.get(), CI.getLangOpts());
+Includes->addSystemHeadersMapping(CI.getLangOpts());
 if (IncludeGraphCallback != nullptr)
   CI.getPreprocessor().addPPCallbacks(
   std::make_unique(CI.getSourceManager(), IG));
Index: clang-tools-extra/clangd/index/CanonicalIncludes.h
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.h
+++ clang-tools-extra/clangd/index/CanonicalIncludes.h
@@ -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 
 #

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

2019-09-09 Thread 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 &Language) {
+  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

2019-09-09 Thread 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

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

2019-09-09 Thread 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&view=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&r1=371407&r2=371408&view=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(&CanonIncludes, Clang->getLangOpts());
+CanonIncludes.addSystemHeadersMapping(Clang->getLangOpts());
   std::unique_ptr IWYUHandler =
   collectIWYUHeaderMaps(&CanonIncludes);
   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&r1=371407&r2=371408&view=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 &CI) override {
-addSystemHeadersMapping(&CanonIncludes, CI.getLangOpts());
+CanonIncludes.addSystemHeadersMapping(CI.getLangOpts());
 SourceMgr = &CI.getSourceManager();
   }
 

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&r1=371407&r2=371408&view=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 = S

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

2019-09-09 Thread 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&id=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 &CI) override {
-addSystemHeadersMapping(&CanonIncludes, CI.getLangOpts());
+CanonIncludes.addSystemHeadersMapping(CI.getLangOpts());
 SourceMgr = &CI.getSourceManager();
   }
 
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(&CanonIncludes, Clang->getLangOpts());
+CanonIncludes.addSystemHeadersMapping(Clang->getLangOpts());
   std::unique_ptr IWYUHandler =
   collectIWYUHeaderMaps(&CanonIncludes);
   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(&CI, 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(&CI, 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(&Includes, Language);
+  Includes.addSystemHeadersMapping(Language);
   CollectorOpts.Includ

r371410 - [NFC] Add aacps bitfields access test

2019-09-09 Thread 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&view=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&view=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] D67341: [clangd] Simplify semantic highlighting visitor

2019-09-09 Thread 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 &TL) {
-// 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 TraverseNestedNameSpec

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

2019-09-09 Thread 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 Microsoft's

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

2019-09-09 Thread 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] D67264: [clangd] Collect location of macro definition in the ParsedAST

2019-09-09 Thread 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 &L : AST.getMainFileExpansions())
+for (const SourceLocation &L : 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] D66564: [clang-tidy] new FPGA struct pack align check

2019-09-09 Thread 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] D67358: Implement semantic selections.

2019-09-09 Thread 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
===
--- clang-tools-extra/clangd/

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

2019-09-09 Thread 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(),
   Res.getPreproce

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

2019-09-09 Thread 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] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

2019-09-09 Thread 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] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

2019-09-09 Thread 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] D67358: Implement semantic selections.

2019-09-09 Thread 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
   SelectionT

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

2019-09-09 Thread 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

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

2019-09-09 Thread 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&view=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&r1=371421&r2=371422&view=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] D66796: [clang] Loop pragma vectorize(disable)

2019-09-09 Thread 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

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

2019-09-09 Thread 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.

2019-09-09 Thread 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] D67079: [analyzer] CastValueChecker: Model inheritance

2019-09-09 Thread 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 {};

1 2 >

1 - 100 of 148 matches

Mail list logo