[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D55994#1343669 , @jkorous wrote:

> This looks like a work around LSP imperfection indeed.
>
> Are you going to push for change in LSP? Something like 
> `CompletionOptions/triggerCharacters` -> `CompletionOptions/triggerStrings`?
>
>   export interface CompletionOptions {
>   /**
>* The server provides support to resolve additional
>* information for a completion item.
>*/
>   resolveProvider?: boolean;
>  
>   /**
>* The characters that trigger completion automatically.
>*/
>   triggerCharacters?: string[];
>   }
>
>
> https://microsoft.github.io/language-server-protocol/specification


I think LSP uses a single character as a trigger character (see 
CompletionTriggerKind::TriggerCharacter). The `string` type of 
`triggerCharacter` is misleading, a more correct type is `char` I believe, but 
`char` is not the basic type in JSON schema.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55994



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

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D55363#1343611 , @jkorous wrote:

> I am a bit late to the discussion but isn't this something to be dealt with 
> on the client's side?


That would mean more complex code in **every** client and more communication 
between the server and the client, which is not particularly useful.
Doing it on the server means keeping the complexity in one place, this looks 
strictly better, unless there are actual use-cases for high-frequency updates.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55363



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

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D55994#1343669 , @jkorous wrote:

> This looks like a work around LSP imperfection indeed.


Totally.

> Are you going to push for change in LSP? Something like 
> `CompletionOptions/triggerCharacters` -> `CompletionOptions/triggerStrings`?

This would workaround this particular problem, but I don't expect the 
`triggerStrings` to cover all the use-cases we care about in the long-term.
A few examples that need semantic knowledge:

- Auto-completions inside a comment

  int test() {
std::^ // <-- should trigger here
// std::^ <-- this is inside a comment, should shouldn't trigger here as 
clang won't provide any useful results anyway.
  }

- Trailing return types

  auto foo() ->^ int {}// <-- should not trigger here


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55994



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

[PATCH] D56226: [clang-format] square parens that are followed by '=' are not Objective-C message sends

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

Hi Alex, thanks for fixing this.

I assume chance of testing an Objective-C header with assignment to method 
result without any other specifically Objective-C (detectable) construct is 
significantly lower than chance of testing a C99 header with designated 
initializers. In other words - having potential for false Objective-C negatives 
in `parseSquare()` is probably fine given the context. Is that right?

If my assumption is correct I'd say LGTM unless anyone says otherwise within 1 
week.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56226



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

[PATCH] D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaDeclCXX.cpp:8175
   DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo();
-  if (FTI.TypeQuals != 0) {
-if (FTI.TypeQuals & Qualifiers::Const)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "const" << SourceRange(D.getIdentifierLoc());
-if (FTI.TypeQuals & Qualifiers::Volatile)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "volatile" << SourceRange(D.getIdentifierLoc());
-if (FTI.TypeQuals & Qualifiers::Restrict)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "restrict" << SourceRange(D.getIdentifierLoc());
+  if (FTI.MethodQualifiers && FTI.MethodQualifiers->getTypeQualifiers() != 0) {
+auto DiagQual = [&](DeclSpec::TQ TypeQual, StringRef QualName,

rjmccall wrote:
> Anastasia wrote:
> > rjmccall wrote:
> > > I think you should add a `hasMethodQualifiers` method to FTI that does 
> > > this check.  Note that it needs to check for attributes, too, and I think 
> > > you need to figure out some way to generalize `forEachCVRUQual` to cover 
> > > those.
> > Are there any attributes I should handle currently?
> > 
> > Also are you suggesting to add another `forEach...` method or extend 
> > existing? If the latter, I might not be able to use it in all places I use 
> > it now.
> Adding another method might be easier.  How many clients actually use the TQ?
In **DeclSpec.cpp** I definitely  need just TQ. I am not sure about 
**SemaType.cpp**. All other places (3x) I guess should be possible to 
generalize. Although I am not very clear if I should be checking all attr. It 
might be a bit exhaustive since the use cases are for the function?

Perhaps, I could add an extra helper `forEachQualifier` that can call 
`forEachCVRUQual` and then I could add a FIXME to complete the rest. We can 
extend it as we discover what's missing. For example I will add address spaces 
there in my next patch. Would this make sense?

As for `hasMethodQualifiers` just to be clear I would need to check for all 
qualifiers including reference qualifier, attributes, etc?


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

https://reviews.llvm.org/D55948



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

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

@hokein I think you are correct. I meant that it would be possible to make LSP 
more generic by using trigger strings instead of trigger characters..
@ilya-biryukov Yes, that's true. I would still expect some performance gain in 
more restrictive filtering on client's side - not sure how big though. Anyway, 
seems like LSP folks think character + trigger context is good enough. For 
example here:
https://github.com/Microsoft/language-server-protocol/issues/138


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55994



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

[PATCH] D56241: expose a control flag for interger to pointer ext warning

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Looks ok in general.
Perhaps there should be a test for this flag specifically?
Also, should "pointer-integer-compare" be in some group?


Repository:
  rC Clang

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

https://reviews.llvm.org/D56241



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

[PATCH] D56263: [clangd] Always try to build absolute path

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

When canonicalizing paths do not look at tryGetRealPathName, which
contains the resolved path for files that are symlinks. Instead first build the
absolute path even if it contains some symlinks on the path. Then resolve only
the smylinks on the path and leave it as it is if the file itself is a symlink.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56263

Files:
  clangd/SourceCode.cpp


Index: clangd/SourceCode.cpp
===
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -187,15 +187,8 @@
const SourceManager &SourceMgr) {
   if (!F)
 return None;
-  // Ideally, we get the real path from the FileEntry object.
-  SmallString<128> FilePath = F->tryGetRealPathName();
-  if (!FilePath.empty() && sys::path::is_absolute(FilePath))
-return FilePath.str().str();
-
-  // Otherwise, we try to compute ourselves.
-  FilePath = F->getName();
-  vlog("FileEntry for {0} did not contain the real path.", FilePath);
 
+  SmallString<128> FilePath = F->getName();
   if (!sys::path::is_absolute(FilePath)) {
 if (auto EC =
 SourceMgr.getFileManager().getVirtualFileSystem()->makeAbsolute(


Index: clangd/SourceCode.cpp
===
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -187,15 +187,8 @@
const SourceManager &SourceMgr) {
   if (!F)
 return None;
-  // Ideally, we get the real path from the FileEntry object.
-  SmallString<128> FilePath = F->tryGetRealPathName();
-  if (!FilePath.empty() && sys::path::is_absolute(FilePath))
-return FilePath.str().str();
-
-  // Otherwise, we try to compute ourselves.
-  FilePath = F->getName();
-  vlog("FileEntry for {0} did not contain the real path.", FilePath);
 
+  SmallString<128> FilePath = F->getName();
   if (!sys::path::is_absolute(FilePath)) {
 if (auto EC =
 SourceMgr.getFileManager().getVirtualFileSystem()->makeAbsolute(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55932: [Sema] Simplfy static_assert diagnostic code.

[Clement Courbet via Phabricator via cfe-commits]

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



Comment at: test/SemaCXX/static-assert.cpp:130
 static_assert(std::is_same()), 
int>::value, "message");
-// expected-error@-1{{static_assert failed due to requirement 
'std::is_same, int>::value' "message"}}
+// expected-error@-1{{static_assert failed due to requirement 
'std::is_same, int>::value' "message"}}
 static_assert(std::is_const::value, "message");

aaron.ballman wrote:
> courbet wrote:
> > aaron.ballman wrote:
> > > courbet wrote:
> > > > aaron.ballman wrote:
> > > > > Any idea why the `std::` was dropped here?
> > > > `NestedNameSpecifier::print()` explicitly does:
> > > > 
> > > > ```
> > > >  PrintingPolicy InnerPolicy(Policy);
> > > >  InnerPolicy.SuppressScope = true;
> > > > ```
> > > > 
> > > Ah, good point, but is that a good behavioral change? I slightly prefer 
> > > printing the namespace name there -- it will likely be redundant 
> > > information most of the time, but when the namespace actually matters, 
> > > having it printed could save someone a lot of head scratching.
> > > I slightly prefer printing the namespace name there
> > 
> > I tend to agree, so it's more a trade-off of code complexity vs better 
> > diagnostic - I tend to err on the side of simplifying the code :)
> > 
> > Another option is to add yet another boolean to PrintingPolicy, but I htink 
> > this is too narrow a use case.
> Heh, I tend to err on the side of helping the user unless the code will be 
> truly awful. I agree that another option on PrintingPolicy may not be the 
> best approach. Do you know why the namespace is being suppressed in the first 
> place? Another option would be to always print the namespace, but I don't 
> know what that might regress (if anything).
Unfortunately always printing the qualification breaks 30 tests, including some 
in a very bad way:

```
/usr/local/google/home/courbet/llvm/llvm/tools/clang/test/AST/ast-print-out-of-line-func.cpp:29:11:
 error: CHECK: expected string not found in input
// CHECK: void Wrapper::Inner::operator+=(int)
  ^
:14:43: note: scanning from here
 ns::Wrapper::ns::Wrapper::Inner::Inner() {
  ^
:16:19: note: possible intended match here
 void ns::Wrapper::ns::Wrapper::Inner::operator+=(int) {
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D55932



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

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Karl-Johan Karlsson via Phabricator via cfe-commits]

Ka-Ka added a comment.

Ping?


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

https://reviews.llvm.org/D17741



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

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Hi Bernhard,

thanks for you patch!
You mentioned that this is your first contribution, if you didn't find these 
docs already they might help you with the LLVM source-code a bit:

- https://llvm.org/docs/ProgrammersManual.html
- https://llvm.org/docs/DeveloperPolicy.html
- https://llvm.org/docs/CodingStandards.html




Comment at: test/clang-tidy/modernize-use-trailing-return.cpp:2
+// RUN: %check_clang_tidy %s modernize-use-trailing-return %t -- -- --std=c++14
+
+namespace std {

bernhardmgruber wrote:
> aaron.ballman wrote:
> > lebedev.ri wrote:
> > > Missing test coverage:
> > > * macros
> > > * is there tests for implicit functions?
> > > * Out-of-line function with body.
> > Also:
> >   * functions with attributes in the type position `int f() [[]];`
> >   * functions without attributes in the type position `[[]] int f();` and 
> > `int f [[]] ();`
> >   * lambdas?
> >   * Special functions without return types, like constructors and 
> > destructors
> >   * Conversion operators. `struct S {  operator int() const; };`
> @lebedev.ri: I do not understand what kind of tests I should write for 
> macros. Do you mean to rewrite functions inside macro definitions as well?
> 
> like rewriting
> ```
> #define DEFINE_F int f();
> ```
> into
> 
> ```
> #define DEFINE_F auto f() -> int;
> ```
> 
> Apart from that, I believe a lot of difficult trickery can be done with 
> macros and I am fine without supporting those (i.e. omitting a possible 
> rewrite, erroneus replacements should never happen). Can you come up with a 
> particular example you would like to be rewritten?
usually we leave code inside macros alone as you said because of all the 
potential issues macros create.
it is best, to cover some expected macro-cases and find a reasonable (sometimes 
configurable) policy for them.

Some things that come to my mind (macro is all upper case):

- `RETURN_TYPE foo();`
- `CONST return_type foo();`
- `ALWAYS_INLINE DLL_EXPORT int foo();` ( probably the same as above)
- `int foo() ANOTHER_ATTRIBUTE;`
- `int FUNCTION_NAME_MACRO(foo, bar)();`
- `FULL_DEFINITION_IN_MACRO(foo, bar);`
- GTest-like function/method creation in bigger macros. Sometimes full classes 
with members are created in macros, in principle the same as the prior case

The safest and easiest way is to bail out anytime you find a macro-id in the 
range you want to transform. Some things, like case 1-5 could in theory be 
transformable, but should we? If you decide to transform them it is necessary 
to not use the expanded macro-tokens but the original source text, which should 
be tested too.



Comment at: test/clang-tidy/modernize-use-trailing-return.cpp:173
+auto l1 = [](int arg) {};
+auto l2 = [](int arg) -> double {};

you could figure out the return type of the lambda if it contains a return, 
otherwise it should be `void`.


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

https://reviews.llvm.org/D56160



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

[PATCH] D55933: [Sema] Do not print default template parameters.

[Clement Courbet via Phabricator via cfe-commits]

courbet updated this revision to Diff 180035.
courbet marked 2 inline comments as done.
courbet added a comment.

Add more tests as suggested in the comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55933

Files:
  include/clang/AST/NestedNameSpecifier.h
  lib/AST/NestedNameSpecifier.cpp
  lib/AST/TypePrinter.cpp
  lib/Sema/SemaTemplate.cpp
  test/SemaCXX/static-assert-cxx17.cpp
  test/SemaCXX/static-assert.cpp

Index: test/SemaCXX/static-assert.cpp
===
--- test/SemaCXX/static-assert.cpp
+++ test/SemaCXX/static-assert.cpp
@@ -127,7 +127,7 @@
 static_assert(!(std::is_const()()), "message");
 // expected-error@-1{{static_assert failed due to requirement '!(std::is_const()())' "message"}}
 static_assert(std::is_same()), int>::value, "message");
-// expected-error@-1{{static_assert failed due to requirement 'std::is_same, int>::value' "message"}}
+// expected-error@-1{{static_assert failed due to requirement 'std::is_same, int>::value' "message"}}
 static_assert(std::is_const::value, "message");
 // expected-error@-1{{static_assert failed due to requirement 'std::is_const::value' "message"}}
 static_assert(std::is_const::value, "message");
Index: test/SemaCXX/static-assert-cxx17.cpp
===
--- test/SemaCXX/static-assert-cxx17.cpp
+++ test/SemaCXX/static-assert-cxx17.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++1z -triple=x86_64-linux-gnu
 
-template 
+template 
 struct S1 {
   static constexpr const bool value = false;
 };
@@ -14,7 +14,7 @@
   static inline constexpr bool var = global_inline_var;
 };
 
-template 
+template 
 inline constexpr bool constexpr_return_false() {
   return false;
 }
@@ -23,6 +23,8 @@
 void foo() {
   static_assert(S1::value);
   // expected-error@-1{{static_assert failed due to requirement 'S1::value'}}
+  static_assert(S1::value);
+  // expected-error@-1{{static_assert failed due to requirement 'S1::value'}}
 }
 template void foo();
 // expected-note@-1{{in instantiation of function template specialization 'foo' requested here}}
@@ -66,7 +68,7 @@
   using U = float;
 };
 
-template 
+template 
 struct X {
   int i = 0;
   int j = 0;
@@ -74,7 +76,7 @@
 };
 
 template 
-void foo6() {
+void test_template_parameter_from_default() {
   static_assert(X());
   // expected-error@-1{{static_assert failed due to requirement 'X()'}}
   static_assert(X{});
@@ -94,11 +96,55 @@
   static_assert(static_cast *>(nullptr));
   // expected-error@-1{{static_assert failed due to requirement 'static_cast *>(nullptr)'}}
   static_assert((const X[]){} == nullptr);
-  // expected-error@-1{{static_assert failed due to requirement '(X const[0]){} == nullptr'}}
+  // expected-error@-1{{static_assert failed due to requirement '(const X [0]){} == nullptr'}}
   static_assert(sizeof(X().X::~X())>) == 0);
   // expected-error@-1{{static_assert failed due to requirement 'sizeof(X) == 0'}}
+  static_assert(constexpr_return_false());
+  // expected-error@-1{{static_assert failed due to requirement 'constexpr_return_false()'}}
+}
+template void test_template_parameter_from_default();
+// expected-note@-1{{in instantiation of function template specialization 'test_template_parameter_from_default' requested here}}
+
+template 
+void test_template_parameter_as_written() {
+  static_assert(X());
+  // expected-error@-1{{static_assert failed due to requirement 'X()'}}
+  static_assert(sizeof(X) == 0);
+  // expected-error@-1{{static_assert failed due to requirement 'sizeof(X) == 0'}}
   static_assert(constexpr_return_false());
   // expected-error@-1{{static_assert failed due to requirement 'constexpr_return_false()'}}
 }
-template void foo6();
-// expected-note@-1{{in instantiation of function template specialization 'foo6' requested here}}
+template void test_template_parameter_as_written();
+// expected-note@-1{{in instantiation of function template specialization 'test_template_parameter_as_written' requested here}}
+
+
+template
+struct is_pointerable { static constexpr bool value = false; };
+
+template
+struct is_pointerable { static constexpr bool value = true; };
+
+template
+void test_is_pointerable()
+{
+static_assert(is_pointerable::value);
+// expected-error@-1{{due to requirement 'is_pointerable::value'}}
+static_assert(not is_pointerable::value);
+// expected-error@-1{{due to requirement '!is_pointerable::value'}}
+}
+template void test_is_pointerable();
+// expected-note@-1{{in instantiation of function template specialization 'test_is_pointerable' requested here}}
+template void test_is_pointerable();
+// expected-note@-1{{in instantiation of function template specialization 'test_is_pointerable' requested here}}
+
+
+// This test emulates std::vector with a default allocator.
+// FIXME: Ideally we would like to avoid printing the default here (print 'S1 *, void>::

[PATCH] D55933: [Sema] Do not print default template parameters.

[Clement Courbet via Phabricator via cfe-commits]

courbet added inline comments.



Comment at: test/SemaCXX/static-assert-cxx17.cpp:109
+template 
+void foo7() {
+  static_assert(X());

Quuxplusone wrote:
> courbet wrote:
> > Quuxplusone wrote:
> > > None of these new test cases actually involve the default template 
> > > argument.
> > > 
> > > I'd like to see two test cases explicitly mimicking `vector`. (Warning: I 
> > > didn't run this code!)
> > > ```
> > > template struct A {};
> > > template> struct V {};
> > > template void testx() {
> > > static_assert(std::is_same::value, "");
> > > // expected-error@-1{{due to requirement 'std::is_same*, 
> > > void>::value'}}
> > > }
> > > template testx>();
> > > template struct PMRA {};
> > > template using PMRV = V>;
> > > template void test() {
> > > static_assert(std::is_same::value, "");
> > > // expected-error@-1{{due to requirement 'std::is_same > > PMRA>*, void>::value'}}
> > > // expected-error@-2{{due to requirement 'std::is_same*, 
> > > void>::value'}}
> > > }
> > > template testy>();
> > > ```
> > > The `expected-error@-2` above is my fantasy world. I don't think it's 
> > > possible to achieve in real life. :)
> > > None of these new test cases actually involve the default template 
> > > argument.
> > 
> > This one is to check that we actually do print when specified explicitly. 
> > foo6 above tests  the default template arguments (notice the change from 
> > `template  struct X to `template  
> > struct X` above). I've renamed `foo6` and `foo7` to make that clear.
> > 
> > Before this change, static_asserts in `foo6` and `foo7` printed the same 
> > thing. Now they don't.
> > 
> > > I'd like to see two test cases explicitly mimicking vector.
> > 
> > OK, I think I misunderstood what you wanted here. I don't think what you 
> > want is actually doable, because by the time you're in `test()`, C is just 
> > a type without any way of knowing whether the user explicitly provided the 
> > template parameter or relied on the default.
> > 
> > What we could do though is **always** erase template parameters that are 
> > the same as default ones. But that would mean printing `due to requirement 
> > 'std::is_same*, void>::value'` even when the user wrote `template 
> > testx>>();`.
> > WDYT ?
> > 
> > 
> Here's what I wrote over on D55270.
> 
> >>! In D55270#1327704, @Quuxplusone wrote:
> > @courbet: On the cpplang Slack, Peter Feichtinger mentioned that defaulted 
> > template arguments should perhaps be treated differently. Is there any way 
> > to suppress the `, std::allocator` part of this diagnostic? 
> > https://godbolt.org/z/TM0UHc
> > 
> > Before your patches:
> > ```
> > :11:5: error: static_assert failed due to requirement 
> > 'std::is_integral_v'
> > static_assert(std::is_integral_v);
> > ^ ~
> > ```
> > After your patches:
> > ```
> > :11:5: error: static_assert failed due to requirement 
> > 'std::is_integral_v > >'
> > static_assert(std::is_integral_v);
> > ^ ~ 
> > ```
> > I don't think the new behavior is //worse//; but I don't think it's 
> > optimal, either.
> > 
> > I think Clang already has a feature to suppress printing these parameters; 
> > you would just have to figure out where it is and try to hook it into your 
> > thing. (And if you do, I'm going to ask for a test case where `T::t` is 
> > `std::pmr::vector`!)
> 
> So does this patch (D55933) actually change the message printed by Peter's 
> https://godbolt.org/z/TM0UHc ? (I think it does not.)
> Does it change the message printed by https://godbolt.org/z/Q0AD70 ? (I think 
> it does.)
> 
> I think most of your new test cases in `foo7` are redundant and don't really 
> need to be there. Contrariwise, I would like to see one new test case 
> isomorphic to https://godbolt.org/z/Q0AD70 , because that strikes me as a 
> very realistic case that we want to protect against regressions.
> So does this patch (D55933) actually change the message printed by Peter's 
> https://godbolt.org/z/TM0UHc ? (I think it does not.)

This is correct, it does not.

> Does it change the message printed by https://godbolt.org/z/Q0AD70 ? (I think 
> it does.)

It does.

> I think most of your new test cases in foo7 are redundant and don't really 
> need to be there. 

Sound good, but I've still kept a few cases for coverage.

> Contrariwise, I would like to see one new test case isomorphic to 
> https://godbolt.org/z/Q0AD70 , because that strikes me as a very realistic 
> case that we want to protect against regressions.

Done.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55933



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

[PATCH] D54309: [AST] Allow limiting the scope of common AST traversals (getParents, RAV).

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

new years ping on the crash-issue.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54309



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

[PATCH] D56267: [clangd] Interfaces for writing code actions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: kadircet, ioeric, hokein, sammccall.
Herald added subscribers: arphaman, jkorous, MaskRay, mgorny.

The code actions can be run in two stages:

- Stage 1. Decides whether the action is available to the user and collects all 
the information required to finish the action. Should be cheap, since this will 
run over all the actions known to clangd on each textDocument/codeAction 
request.
- Stage 2. Uses information from stage 1 to produce the actual edits that the 
code action should perform. This stage can be expensive and will only run if 
the user chooses to perform the specified action in the UI.

Trivial actions can also produce results in stage 1 to avoid an extra
round-trip and read of the AST to reconstruct the action on the server.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56267

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/CodeActions.cpp
  clangd/CodeActions.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -25,7 +25,8 @@
 # CHECK-NEXT:  "documentSymbolProvider": true,
 # CHECK-NEXT:  "executeCommandProvider": {
 # CHECK-NEXT:"commands": [
-# CHECK-NEXT:  "clangd.applyFix"
+# CHECK-NEXT:  "clangd.applyFix",
+# CHECK-NEXT:  "clangd.applyCodeAction"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "hoverProvider": true,
Index: clangd/SourceCode.h
===
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -65,6 +65,10 @@
 std::pair offsetToClangLineColumn(llvm::StringRef Code,
   size_t Offset);
 
+/// Return the substring, corresponding to the passed range.
+llvm::Expected rangeSubstr(llvm::StringRef Code,
+const Range &R);
+
 /// From "a::b::c", return {"a::b::", "c"}. Scope is empty if there's no
 /// qualifier.
 std::pair
Index: clangd/SourceCode.cpp
===
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -161,6 +161,20 @@
   return {Lines + 1, Offset - StartOfLine + 1};
 }
 
+llvm::Expected rangeSubstr(llvm::StringRef Code,
+const Range &R) {
+  auto Begin = positionToOffset(Code, R.start, false);
+  if (!Begin)
+return Begin.takeError();
+  auto End = positionToOffset(Code, R.end, false);
+  if (!End)
+return End.takeError();
+  if (*End < *Begin)
+return createStringError(inconvertibleErrorCode(),
+ "invalid range passed to rangeSubstr");
+  return Code.substr(*Begin, *End - *Begin);
+}
+
 std::pair splitQualifiedName(StringRef QName) {
   size_t Pos = QName.rfind("::");
   if (Pos == StringRef::npos)
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -632,6 +632,14 @@
 bool fromJSON(const llvm::json::Value &, WorkspaceEdit &);
 llvm::json::Value toJSON(const WorkspaceEdit &WE);
 
+struct CodeActionArgs {
+  std::string file;
+  int64_t actionId;
+  Range selection;
+};
+bool fromJSON(const llvm::json::Value &, CodeActionArgs &);
+llvm::json::Value toJSON(const CodeActionArgs &A);
+
 /// Exact commands are not specified in the protocol so we define the
 /// ones supported by Clangd here. The protocol specifies the command arguments
 /// to be "any[]" but to make this safer and more manageable, each command we
@@ -643,12 +651,15 @@
 struct ExecuteCommandParams {
   // Command to apply fix-its. Uses WorkspaceEdit as argument.
   const static llvm::StringLiteral CLANGD_APPLY_FIX_COMMAND;
+  // Command to apply the code action. Uses action id as the argument.
+  const static llvm::StringLiteral CLANGD_APPLY_CODE_ACTION;
 
   /// The command identifier, e.g. CLANGD_APPLY_FIX_COMMAND
   std::string command;
 
   // Arguments
   llvm::Optional workspaceEdit;
+  llvm::Optional codeActionArgs;
 };
 bool fromJSON(const llvm::json::Value &, ExecuteCommandParams &);
 
@@ -670,6 +681,7 @@
   /// Used to filter code actions.
   llvm::Optional kind;
   const static llvm::StringLiteral QUICKFIX_KIND;
+  const static llvm::StringLiteral REFACTOR_KIND;
 
   /// The diagnostics that this code action resolves.
   llvm::Optional> diagnostics;
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -418,6 +418,9 @@
 
 const StringLiteral ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND =
 "clangd.applyFix";
+const StringLiteral ExecuteCommandParams::CLANGD_

[PATCH] D56267: [clangd] Interfaces for writing code actions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

That's the result of me playing around with writing a small code action for 
clangd.
It's not final yet, e.g. missing tests and example actions, but still wanted to 
drop the change here to make sure it's not lost, I believe we'll need something 
like this in the near future anyway.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56267



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

[clang-tools-extra] r350303 - [clangd] clang-format everything. NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Jan  3 05:28:05 2019
New Revision: 350303

URL: http://llvm.org/viewvc/llvm-project?rev=350303&view=rev
Log:
[clangd] clang-format everything. NFC

Modified:
clang-tools-extra/trunk/clangd/AST.cpp
clang-tools-extra/trunk/clangd/AST.h
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
clang-tools-extra/trunk/clangd/Diagnostics.cpp
clang-tools-extra/trunk/clangd/FSProvider.cpp
clang-tools-extra/trunk/clangd/FileDistance.h
clang-tools-extra/trunk/clangd/FindSymbols.cpp
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/clangd/Trace.cpp
clang-tools-extra/trunk/clangd/URI.cpp
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/JSONTransportTests.cpp
clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolInfoTests.cpp
clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/AST.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.cpp?rev=350303&r1=350302&r2=350303&view=diff
==
--- clang-tools-extra/trunk/clangd/AST.cpp (original)
+++ clang-tools-extra/trunk/clangd/AST.cpp Thu Jan  3 05:28:05 2019
@@ -44,7 +44,7 @@ bool isSpelledInSourceCode(const Decl *D
 
 bool isImplementationDetail(const Decl *D) { return !isSpelledInSourceCode(D); 
}
 
-SourceLocation findNameLoc(const clang::Decl* D) {
+SourceLocation findNameLoc(const clang::Decl *D) {
   const auto &SM = D->getASTContext().getSourceManager();
   if (!isSpelledInSourceCode(D))
 // Use the expansion location as spelling location is not interesting.

Modified: clang-tools-extra/trunk/clangd/AST.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.h?rev=350303&r1=350302&r2=350303&view=diff
==
--- clang-tools-extra/trunk/clangd/AST.h (original)
+++ clang-tools-extra/trunk/clangd/AST.h Thu Jan  3 05:28:05 2019
@@ -14,9 +14,9 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
 
+#include "index/Index.h"
 #include "clang/AST/Decl.h"
 #include "clang/Basic/SourceLocation.h"
-#include "index/Index.h"
 
 namespace clang {
 class SourceManager;

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=350303&r1=350302&r2=350303&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Jan  3 05:28:05 2019
@@ -163,9 +163,9 @@ private:
   Server(Other.Server), TraceArgs(Other.TraceArgs) {
   Other.Server = nullptr;
 }
-ReplyOnce& operator=(ReplyOnce&&) = delete;
+ReplyOnce &operator=(ReplyOnce &&) = delete;
 ReplyOnce(const ReplyOnce &) = delete;
-ReplyOnce& operator=(const ReplyOnce&) = delete;
+ReplyOnce &operator=(const ReplyOnce &) = delete;
 
 ~ReplyOnce() {
   if (Server && !Replied) {
@@ -614,23 +614,23 @@ void ClangdLSPServer::onCodeAction(const
 
 void ClangdLSPServer::onCompletion(const TextDocumentPositionParams &Params,
Callback Reply) {
-  Server->codeComplete(Params.textDocument.uri.file(), Params.position, CCOpts,
-   Bind(
-   [this](decltype(Reply) Reply,
-  Expected List) {
- if (!List)
-   return Reply(List.takeError());
- CompletionList LSPList;
- LSPList.isIncomplete = List->HasMore;
- for (const auto &R : List->Completions) {
-   CompletionItem C = R.render(CCOpts);
-   C.kind = adjustKindToCapab

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D55994#1344717 , @jkorous wrote:

> @ilya-biryukov Yes, that's true. I would still expect some performance gain 
> in more restrictive filtering on client's side - not sure how big though. 
> Anyway, seems like LSP folks think character + trigger context is good 
> enough. For example here:
>  https://github.com/Microsoft/language-server-protocol/issues/138


That would definitely mean less completion requests from the client, I doubt it 
would be the bottleneck for performance, though.
Thanks for the pointers, an insightful conversation on almost the same issue 
there.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55994



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

[PATCH] D55994: [clangd] Check preceding char when completion triggers on ':' or '>'

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350304: [clangd] Check preceding char when completion 
triggers on ':' or '>' (authored by ibiryukov, committed 
by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55994?vs=179844&id=180042#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55994

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdLSPServer.h
  clang-tools-extra/trunk/clangd/Protocol.cpp
  clang-tools-extra/trunk/clangd/Protocol.h
  clang-tools-extra/trunk/test/clangd/completion-auto-trigger.test

Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -22,6 +22,15 @@
 namespace clang {
 namespace clangd {
 namespace {
+class IgnoreCompletionError : public llvm::ErrorInfo {
+public:
+  void log(llvm::raw_ostream &OS) const override {
+OS << "ignored auto-triggered completion, preceding char did not match";
+  }
+  std::error_code convertToErrorCode() const override {
+return std::make_error_code(std::errc::operation_canceled);
+  }
+};
 
 void adjustSymbolKinds(llvm::MutableArrayRef Syms,
SymbolKindBitset Kinds) {
@@ -310,6 +319,8 @@
 {"completionProvider",
  json::Object{
  {"resolveProvider", false},
+ // We do extra checks for '>' and ':' in completion to only
+ // trigger on '->' and '::'.
  {"triggerCharacters", {".", ">", ":"}},
  }},
 {"signatureHelpProvider",
@@ -612,8 +623,10 @@
   }
 }
 
-void ClangdLSPServer::onCompletion(const TextDocumentPositionParams &Params,
+void ClangdLSPServer::onCompletion(const CompletionParams &Params,
Callback Reply) {
+  if (!shouldRunCompletion(Params))
+return Reply(llvm::make_error());
   Server->codeComplete(
   Params.textDocument.uri.file(), Params.position, CCOpts,
   Bind(
@@ -773,6 +786,41 @@
   return FixItsIter->second;
 }
 
+bool ClangdLSPServer::shouldRunCompletion(
+const CompletionParams &Params) const {
+  StringRef Trigger = Params.context.triggerCharacter;
+  if (Params.context.triggerKind != CompletionTriggerKind::TriggerCharacter ||
+  (Trigger != ">" && Trigger != ":"))
+return true;
+
+  auto Code = DraftMgr.getDraft(Params.textDocument.uri.file());
+  if (!Code)
+return true; // completion code will log the error for untracked doc.
+
+  // A completion request is sent when the user types '>' or ':', but we only
+  // want to trigger on '->' and '::'. We check the preceeding character to make
+  // sure it matches what we expected.
+  // Running the lexer here would be more robust (e.g. we can detect comments
+  // and avoid triggering completion there), but we choose to err on the side
+  // of simplicity here.
+  auto Offset = positionToOffset(*Code, Params.position,
+ /*AllowColumnsBeyondLineLength=*/false);
+  if (!Offset) {
+vlog("could not convert position '{0}' to offset for file '{1}'",
+ Params.position, Params.textDocument.uri.file());
+return true;
+  }
+  if (*Offset < 2)
+return false;
+
+  if (Trigger == ">")
+return (*Code)[*Offset - 2] == '-'; // trigger only on '->'.
+  if (Trigger == ":")
+return (*Code)[*Offset - 2] == ':'; // trigger only on '::'.
+  assert(false && "unhandled trigger character");
+  return true;
+}
+
 void ClangdLSPServer::onDiagnosticsReady(PathRef File,
  std::vector Diagnostics) {
   auto URI = URIForFile::canonicalize(File, /*TUPath=*/File);
Index: clang-tools-extra/trunk/clangd/Protocol.h
===
--- clang-tools-extra/trunk/clangd/Protocol.h
+++ clang-tools-extra/trunk/clangd/Protocol.h
@@ -775,6 +775,31 @@
 };
 bool fromJSON(const llvm::json::Value &, TextDocumentPositionParams &);
 
+enum class CompletionTriggerKind {
+  /// Completion was triggered by typing an identifier (24x7 code
+  /// complete), manual invocation (e.g Ctrl+Space) or via API.
+  Invoked = 1,
+  /// Completion was triggered by a trigger character specified by
+  /// the `triggerCharacters` properties of the `CompletionRegistrationOptions`.
+  TriggerCharacter = 2,
+  /// Completion was re-triggered as the current completion list is incomplete.
+  TriggerTriggerForIncompleteCompletions = 3
+};
+
+struct CompletionContext {
+  /// How the completion was triggered.
+  CompletionTriggerKind triggerKind = CompletionTriggerKind::Invoked;
+  /// The trigger character (a single character) that has trigger code complete.
+  /// Is undefined if `triggerKind !== CompletionTrigge

[clang-tools-extra] r350304 - [clangd] Check preceding char when completion triggers on ':' or '>'

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Jan  3 05:37:12 2019
New Revision: 350304

URL: http://llvm.org/viewvc/llvm-project?rev=350304&view=rev
Log:
[clangd] Check preceding char when completion triggers on ':' or '>'

Summary:
Only run completion when we were trigerred on '->' and '::', otherwise
send an error code in return.
To avoid automatically invoking completions in cases like 'a >^' or
'a ? b :^'.

Reviewers: hokein

Reviewed By: hokein

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

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

Added:
clang-tools-extra/trunk/test/clangd/completion-auto-trigger.test
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=350304&r1=350303&r2=350304&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Jan  3 05:37:12 2019
@@ -22,6 +22,15 @@ using namespace llvm;
 namespace clang {
 namespace clangd {
 namespace {
+class IgnoreCompletionError : public llvm::ErrorInfo {
+public:
+  void log(llvm::raw_ostream &OS) const override {
+OS << "ignored auto-triggered completion, preceding char did not match";
+  }
+  std::error_code convertToErrorCode() const override {
+return std::make_error_code(std::errc::operation_canceled);
+  }
+};
 
 void adjustSymbolKinds(llvm::MutableArrayRef Syms,
SymbolKindBitset Kinds) {
@@ -310,6 +319,8 @@ void ClangdLSPServer::onInitialize(const
 {"completionProvider",
  json::Object{
  {"resolveProvider", false},
+ // We do extra checks for '>' and ':' in completion to only
+ // trigger on '->' and '::'.
  {"triggerCharacters", {".", ">", ":"}},
  }},
 {"signatureHelpProvider",
@@ -612,8 +623,10 @@ void ClangdLSPServer::onCodeAction(const
   }
 }
 
-void ClangdLSPServer::onCompletion(const TextDocumentPositionParams &Params,
+void ClangdLSPServer::onCompletion(const CompletionParams &Params,
Callback Reply) {
+  if (!shouldRunCompletion(Params))
+return Reply(llvm::make_error());
   Server->codeComplete(
   Params.textDocument.uri.file(), Params.position, CCOpts,
   Bind(
@@ -773,6 +786,41 @@ std::vector ClangdLSPServer::getFix
   return FixItsIter->second;
 }
 
+bool ClangdLSPServer::shouldRunCompletion(
+const CompletionParams &Params) const {
+  StringRef Trigger = Params.context.triggerCharacter;
+  if (Params.context.triggerKind != CompletionTriggerKind::TriggerCharacter ||
+  (Trigger != ">" && Trigger != ":"))
+return true;
+
+  auto Code = DraftMgr.getDraft(Params.textDocument.uri.file());
+  if (!Code)
+return true; // completion code will log the error for untracked doc.
+
+  // A completion request is sent when the user types '>' or ':', but we only
+  // want to trigger on '->' and '::'. We check the preceeding character to 
make
+  // sure it matches what we expected.
+  // Running the lexer here would be more robust (e.g. we can detect comments
+  // and avoid triggering completion there), but we choose to err on the side
+  // of simplicity here.
+  auto Offset = positionToOffset(*Code, Params.position,
+ /*AllowColumnsBeyondLineLength=*/false);
+  if (!Offset) {
+vlog("could not convert position '{0}' to offset for file '{1}'",
+ Params.position, Params.textDocument.uri.file());
+return true;
+  }
+  if (*Offset < 2)
+return false;
+
+  if (Trigger == ">")
+return (*Code)[*Offset - 2] == '-'; // trigger only on '->'.
+  if (Trigger == ":")
+return (*Code)[*Offset - 2] == ':'; // trigger only on '::'.
+  assert(false && "unhandled trigger character");
+  return true;
+}
+
 void ClangdLSPServer::onDiagnosticsReady(PathRef File,
  std::vector Diagnostics) {
   auto URI = URIForFile::canonicalize(File, /*TUPath=*/File);

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=350304&r1=350303&r2=350304&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Thu Jan  3 05:37:12 2019
@@ -74,8 +74,7 @@ private:
   void onDocumentSymbol(const DocumentSymbolParams &,
 Callback);
   void onCodeAction(const CodeActionParams &, Callback);
-  void onCompletion(const TextDocument

[PATCH] D56263: [clangd] Always try to build absolute path

[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
NIT: a typo in the change description: s/smylinks/symlinks

Maybe also add a separate sentence to the description that this should not 
affect the behavior if file is not a symlink, e.g. something like

  This only changes behavior in cases when the file itself is a symlink.

This information is present in the change description, but a little hard to 
find.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56263



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

r350305 - [NewPM] Port Msan

[Philip Pfaffe via cfe-commits]

Author: pfaffe
Date: Thu Jan  3 05:42:44 2019
New Revision: 350305

URL: http://llvm.org/viewvc/llvm-project?rev=350305&view=rev
Log:
[NewPM] Port Msan

Summary:
Keeping msan a function pass requires replacing the module level initialization:
That means, don't define a ctor function which calls __msan_init, instead just
declare the init function at the first access, and add that to the global ctors
list.

Changes:
- Pull the actual sanitizer and the wrapper pass apart.
- Add a newpm msan pass. The function pass inserts calls to runtime
  library functions, for which it inserts declarations as necessary.
- Update tests.

Caveats:
- There is one test that I dropped, because it specifically tested the
  definition of the ctor.

Reviewers: chandlerc, fedor.sergeev, leonardchan, vitalybuka

Subscribers: sdardis, nemanjai, javed.absar, hiraditya, kbarton, bollu, 
atanasyan, jsji

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

Modified:
cfe/trunk/lib/CodeGen/BackendUtil.cpp

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=350305&r1=350304&r2=350305&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Thu Jan  3 05:42:44 2019
@@ -53,6 +53,7 @@
 #include "llvm/Transforms/IPO/ThinLTOBitcodeWriter.h"
 #include "llvm/Transforms/InstCombine/InstCombine.h"
 #include "llvm/Transforms/Instrumentation.h"
+#include "llvm/Transforms/Instrumentation/MemorySanitizer.h"
 #include "llvm/Transforms/Instrumentation/BoundsChecking.h"
 #include "llvm/Transforms/Instrumentation/GCOVProfiler.h"
 #include "llvm/Transforms/ObjCARC.h"
@@ -276,7 +277,7 @@ static void addGeneralOptsForMemorySanit
   const CodeGenOptions &CGOpts = BuilderWrapper.getCGOpts();
   int TrackOrigins = CGOpts.SanitizeMemoryTrackOrigins;
   bool Recover = CGOpts.SanitizeRecover.has(SanitizerKind::Memory);
-  PM.add(createMemorySanitizerPass(TrackOrigins, Recover, CompileKernel));
+  PM.add(createMemorySanitizerLegacyPassPass(TrackOrigins, Recover, 
CompileKernel));
 
   // MemorySanitizer inserts complex instrumentation that mostly follows
   // the logic of the original code, but operates on "shadow" values.


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

[clang-tools-extra] r350306 - [clangd] Always try to build absolute path

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Jan  3 05:46:10 2019
New Revision: 350306

URL: http://llvm.org/viewvc/llvm-project?rev=350306&view=rev
Log:
[clangd] Always try to build absolute path

Summary:
This only changes behavior in cases when the file itself is a symlink.

When canonicalizing paths do not look at tryGetRealPathName, which
contains the resolved path for files that are symlinks. Instead first build the
absolute path even if it contains some symlinks on the path. Then resolve only
the symlinks on the path and leave it as it is if the file itself is a symlink.

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

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

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

Modified:
clang-tools-extra/trunk/clangd/SourceCode.cpp

Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=350306&r1=350305&r2=350306&view=diff
==
--- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp Thu Jan  3 05:46:10 2019
@@ -187,15 +187,8 @@ Optional getCanonicalPath(c
const SourceManager &SourceMgr) {
   if (!F)
 return None;
-  // Ideally, we get the real path from the FileEntry object.
-  SmallString<128> FilePath = F->tryGetRealPathName();
-  if (!FilePath.empty() && sys::path::is_absolute(FilePath))
-return FilePath.str().str();
-
-  // Otherwise, we try to compute ourselves.
-  FilePath = F->getName();
-  vlog("FileEntry for {0} did not contain the real path.", FilePath);
 
+  SmallString<128> FilePath = F->getName();
   if (!sys::path::is_absolute(FilePath)) {
 if (auto EC =
 SourceMgr.getFileManager().getVirtualFileSystem()->makeAbsolute(


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

[PATCH] D56263: [clangd] Always try to build absolute path

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350306: [clangd] Always try to build absolute path (authored 
by kadircet, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56263

Files:
  clang-tools-extra/trunk/clangd/SourceCode.cpp


Index: clang-tools-extra/trunk/clangd/SourceCode.cpp
===
--- clang-tools-extra/trunk/clangd/SourceCode.cpp
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp
@@ -187,15 +187,8 @@
const SourceManager &SourceMgr) {
   if (!F)
 return None;
-  // Ideally, we get the real path from the FileEntry object.
-  SmallString<128> FilePath = F->tryGetRealPathName();
-  if (!FilePath.empty() && sys::path::is_absolute(FilePath))
-return FilePath.str().str();
-
-  // Otherwise, we try to compute ourselves.
-  FilePath = F->getName();
-  vlog("FileEntry for {0} did not contain the real path.", FilePath);
 
+  SmallString<128> FilePath = F->getName();
   if (!sys::path::is_absolute(FilePath)) {
 if (auto EC =
 SourceMgr.getFileManager().getVirtualFileSystem()->makeAbsolute(


Index: clang-tools-extra/trunk/clangd/SourceCode.cpp
===
--- clang-tools-extra/trunk/clangd/SourceCode.cpp
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp
@@ -187,15 +187,8 @@
const SourceManager &SourceMgr) {
   if (!F)
 return None;
-  // Ideally, we get the real path from the FileEntry object.
-  SmallString<128> FilePath = F->tryGetRealPathName();
-  if (!FilePath.empty() && sys::path::is_absolute(FilePath))
-return FilePath.str().str();
-
-  // Otherwise, we try to compute ourselves.
-  FilePath = F->getName();
-  vlog("FileEntry for {0} did not contain the real path.", FilePath);
 
+  SmallString<128> FilePath = F->getName();
   if (!sys::path::is_absolute(FilePath)) {
 if (auto EC =
 SourceMgr.getFileManager().getVirtualFileSystem()->makeAbsolute(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56061: [clang-tools-extra] [clangd] Fix detecting atomics in stand-alone builds

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM. Thanks for the fix.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56061



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D56113#1344210 , @jdenny wrote:

> In D56113#1342879 , @ABataev wrote:
>
> > But you will need another serie of patches for `reduction` and `linear` 
> > clauses to update their error messages.
>
>
> Those already had their own checks for const.  I'm not aware of any cases not 
> handled, and the test suite does pass after my patch.


Yes, they have the checks for constness, but you need to  update those checks 
to emit this new error message for const values with mutable fields.

> By the way, is there any value to keeping the predetermined shared for const 
> if -openmp-version=3.1 or earlier?

Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 it will 
have the value `31`.

>>> Sorry, I misunderstood your first response.  When I try an atomic write to 
>>> the shared variable, the difference between the uncombined and combined 
>>> target teams directive affects functionality when targeting multicore.  
>>> Does that matter?
>> 
>> Yes, I'm aware if this problem. But I don't think it is very important and 
>> can cause serious problems in user code. If you think that it is worth to 
>> fix this prblem, you can go ahead and fix it.
> 
> No, not worthwhile for me right now.  I just wanted to point it out in 
> passing in case it might matter to someone.  Does a bugzilla seem worthwhile 
> to you?

Yes, generally speaking it would be good to fix this. So, yes, at least to keep 
tracking, add a bug report to the bugzilla.


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

https://reviews.llvm.org/D56113



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

r350317 - Diagnose an unused result from a call through a function pointer whose return type is marked [[nodiscard]].

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Thu Jan  3 06:24:31 2019
New Revision: 350317

URL: http://llvm.org/viewvc/llvm-project?rev=350317&view=rev
Log:
Diagnose an unused result from a call through a function pointer whose return 
type is marked [[nodiscard]].

When a function returns a type and that type was declared [[nodiscard]], we 
diagnose any unused results from that call as though the function were marked 
nodiscard. The same behavior should apply to calls through a function pointer.

This addresses PR31526.

Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=350317&r1=350316&r2=350317&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Thu Jan  3 06:24:31 2019
@@ -2327,14 +2327,6 @@ public:
 getASTContext());
   }
 
-  /// Returns the WarnUnusedResultAttr that is either declared on this
-  /// function, or its return type declaration.
-  const Attr *getUnusedResultAttr() const;
-
-  /// Returns true if this function or its return type has the
-  /// warn_unused_result attribute.
-  bool hasUnusedResultAttr() const { return getUnusedResultAttr() != nullptr; }
-
   /// Returns the storage class as written in the source. For the
   /// computed linkage of symbol, see getLinkage.
   StorageClass getStorageClass() const {

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=350317&r1=350316&r2=350317&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Jan  3 06:24:31 2019
@@ -2624,6 +2624,15 @@ public:
   /// type.
   QualType getCallReturnType(const ASTContext &Ctx) const;
 
+  /// Returns the WarnUnusedResultAttr that is either declared on the called
+  /// function, or its return type declaration.
+  const Attr *getUnusedResultAttr(const ASTContext &Ctx) const;
+
+  /// Returns true if this call expression should warn on unused results.
+  bool hasUnusedResultAttr(const ASTContext &Ctx) const {
+return getUnusedResultAttr(Ctx) != nullptr;
+  }
+
   SourceLocation getRParenLoc() const { return RParenLoc; }
   void setRParenLoc(SourceLocation L) { RParenLoc = L; }
 

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=350317&r1=350316&r2=350317&view=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Thu Jan  3 06:24:31 2019
@@ -3231,20 +3231,6 @@ SourceRange FunctionDecl::getExceptionSp
   return FTL.getExceptionSpecRange();
 }
 
-const Attr *FunctionDecl::getUnusedResultAttr() const {
-  QualType RetType = getReturnType();
-  if (const auto *Ret = RetType->getAsRecordDecl()) {
-if (const auto *R = Ret->getAttr())
-  return R;
-  } else if (const auto *ET = RetType->getAs()) {
-if (const EnumDecl *ED = ET->getDecl()) {
-  if (const auto *R = ED->getAttr())
-return R;
-}
-  }
-  return getAttr();
-}
-
 /// For an inline function definition in C, or for a gnu_inline function
 /// in C++, determine whether the definition will be externally visible.
 ///

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=350317&r1=350316&r2=350317&view=diff
==
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Thu Jan  3 06:24:31 2019
@@ -1412,6 +1412,19 @@ QualType CallExpr::getCallReturnType(con
   return FnType->getReturnType();
 }
 
+const Attr *CallExpr::getUnusedResultAttr(const ASTContext &Ctx) const {
+  // If the return type is a struct, union, or enum that is marked nodiscard,
+  // then return the return type attribute.
+  if (const TagDecl *TD = getCallReturnType(Ctx)->getAsTagDecl())
+if (const auto *A = TD->getAttr())
+  return A;
+
+  // Otherwise, see if the callee is marked nodiscard and return that attribute
+  // instead.
+  const Decl *D = getCalleeDecl();
+  return D ? D->getAttr() : nullptr;
+}
+
 SourceLocation CallExpr::getBeginLoc() const {
   if (isa(this))
 return cast(this)->getBeginLoc();
@@ -2323,16 +2336,12 @@ bool Expr::isUnusedResultAWarning(const
 // If this is a direct call, get the callee.
 const CallExpr *CE = cast(this);
 if (const Decl *FD = CE->getCalleeDecl()) {
-  const FunctionDecl *Func = dyn_cast(FD);
-  bool HasWarnUnusedResultAttr = Func ? Func->hasUnusedR

[PATCH] D55949: Correctly handle function pointers returning a type marked nodiscard

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Committed in r350317.


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

https://reviews.llvm.org/D55949



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

r350318 - Portable Python script across Python version

[Serge Guelton via cfe-commits]

Author: serge_sans_paille
Date: Thu Jan  3 06:26:56 2019
New Revision: 350318

URL: http://llvm.org/viewvc/llvm-project?rev=350318&view=rev
Log:
Portable Python script across Python version

StringIO is obsoleted in Python3, replaced by io.BytesIO or io.StringIO 
depending on the use.

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

Modified:
cfe/trunk/bindings/python/tests/cindex/test_translation_unit.py
cfe/trunk/tools/clang-format/clang-format-diff.py
cfe/trunk/tools/scan-view/share/ScanView.py

Modified: cfe/trunk/bindings/python/tests/cindex/test_translation_unit.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/tests/cindex/test_translation_unit.py?rev=350318&r1=350317&r2=350318&view=diff
==
--- cfe/trunk/bindings/python/tests/cindex/test_translation_unit.py (original)
+++ cfe/trunk/bindings/python/tests/cindex/test_translation_unit.py Thu Jan  3 
06:26:56 2019
@@ -6,6 +6,7 @@ if 'CLANG_LIBRARY_PATH' in os.environ:
 from contextlib import contextmanager
 import gc
 import os
+import sys
 import tempfile
 import unittest
 
@@ -93,10 +94,10 @@ int SOME_DEFINE;
 self.assertEqual(spellings[-1], 'y')
 
 def test_unsaved_files_2(self):
-try:
-from StringIO import StringIO
-except:
+if sys.version_info.major >= 3:
 from io import StringIO
+else:
+from io import BytesIO as StringIO
 tu = TranslationUnit.from_source('fake.c', unsaved_files = [
 ('fake.c', StringIO('int x;'))])
 spellings = [c.spelling for c in tu.cursor.get_children()]

Modified: cfe/trunk/tools/clang-format/clang-format-diff.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format-diff.py?rev=350318&r1=350317&r2=350318&view=diff
==
--- cfe/trunk/tools/clang-format/clang-format-diff.py (original)
+++ cfe/trunk/tools/clang-format/clang-format-diff.py Thu Jan  3 06:26:56 2019
@@ -28,10 +28,11 @@ import difflib
 import re
 import subprocess
 import sys
-try:
-  from StringIO import StringIO
-except ImportError:
-   from io import StringIO
+
+if sys.version_info.major >= 3:
+from io import StringIO
+else:
+from io import BytesIO as StringIO
 
 
 def main():

Modified: cfe/trunk/tools/scan-view/share/ScanView.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-view/share/ScanView.py?rev=350318&r1=350317&r2=350318&view=diff
==
--- cfe/trunk/tools/scan-view/share/ScanView.py (original)
+++ cfe/trunk/tools/scan-view/share/ScanView.py Thu Jan  3 06:26:56 2019
@@ -13,7 +13,12 @@ except ImportError:
 from urllib.parse import urlparse, unquote
 
 import posixpath
-import StringIO
+
+if sys.version_info.major >= 3:
+from io import StringIO, BytesIO
+else:
+from io import BytesIO, BytesIO as StringIO
+
 import re
 import shutil
 import threading
@@ -117,7 +122,7 @@ class ReporterThread(threading.Thread):
 except Reporter.ReportFailure as e:
 self.status = e.value
 except Exception as e:
-s = StringIO.StringIO()
+s = StringIO()
 import traceback
 print('Unhandled Exception', file=s)
 traceback.print_exc(file=s)
@@ -275,7 +280,7 @@ class ScanViewRequestHandler(SimpleHTTPR
 
 def handle_exception(self, exc):
 import traceback
-s = StringIO.StringIO()
+s = StringIO()
 print("INTERNAL ERROR\n", file=s)
 traceback.print_exc(file=s)
 f = self.send_string(s.getvalue(), 'text/plain')
@@ -739,15 +744,16 @@ File Bug
 return f
 
 def send_string(self, s, ctype='text/html', headers=True, mtime=None):
+encoded_s = s.encode()
 if headers:
 self.send_response(200)
 self.send_header("Content-type", ctype)
-self.send_header("Content-Length", str(len(s)))
+self.send_header("Content-Length", str(len(encoded_s)))
 if mtime is None:
 mtime = self.dynamic_mtime
 self.send_header("Last-Modified", self.date_time_string(mtime))
 self.end_headers()
-return StringIO.StringIO(s)
+return BytesIO(encoded_s)
 
 def send_patched_file(self, path, ctype):
 # Allow a very limited set of variables. This is pretty gross.


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

r350319 - Portable Python script across Python version

[Serge Guelton via cfe-commits]

Author: serge_sans_paille
Date: Thu Jan  3 06:27:05 2019
New Revision: 350319

URL: http://llvm.org/viewvc/llvm-project?rev=350319&view=rev
Log:
Portable Python script across Python version

Get rid of Python version specific shebang.

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

Modified:
cfe/trunk/utils/check_cfc/check_cfc.py
cfe/trunk/utils/check_cfc/obj_diff.py
cfe/trunk/utils/check_cfc/test_check_cfc.py

Modified: cfe/trunk/utils/check_cfc/check_cfc.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/check_cfc/check_cfc.py?rev=350319&r1=350318&r2=350319&view=diff
==
--- cfe/trunk/utils/check_cfc/check_cfc.py (original)
+++ cfe/trunk/utils/check_cfc/check_cfc.py Thu Jan  3 06:27:05 2019
@@ -1,4 +1,4 @@
-#!/usr/bin/env python2.7
+#!/usr/bin/env python
 
 """Check CFC - Check Compile Flow Consistency
 

Modified: cfe/trunk/utils/check_cfc/obj_diff.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/check_cfc/obj_diff.py?rev=350319&r1=350318&r2=350319&view=diff
==
--- cfe/trunk/utils/check_cfc/obj_diff.py (original)
+++ cfe/trunk/utils/check_cfc/obj_diff.py Thu Jan  3 06:27:05 2019
@@ -1,4 +1,4 @@
-#!/usr/bin/env python2.7
+#!/usr/bin/env python
 
 from __future__ import absolute_import, division, print_function
 

Modified: cfe/trunk/utils/check_cfc/test_check_cfc.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/check_cfc/test_check_cfc.py?rev=350319&r1=350318&r2=350319&view=diff
==
--- cfe/trunk/utils/check_cfc/test_check_cfc.py (original)
+++ cfe/trunk/utils/check_cfc/test_check_cfc.py Thu Jan  3 06:27:05 2019
@@ -1,4 +1,4 @@
-#!/usr/bin/env python2.7
+#!/usr/bin/env python
 
 """Test internal functions within check_cfc.py."""
 


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

[PATCH] D56226: [clang-format] square parens that are followed by '=' are not Objective-C message sends

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

Note that a message send needs to have two expressions without an operator in 
between.  Can we also rule out all square brackets that just have a single 
identifier or number token?


Repository:
  rC Clang

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

https://reviews.llvm.org/D56226



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

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

I like the idea of just getting the filename reliably, but I think an extension 
to strip path prefixes is a bit of an overkill especially as yet another 
compiler flag. I implemented a similar thing in my fork in form of a directive 
to `__generate(...)` which is my own weird extension to do meta-programming 
using the preprocessor, that's the implementation with no extra parameters 
(just as an idea, I know the implementation is not great):

  else if (PS == kMPS_DirectiveFile)
   {
 PresumedLoc PLoc = SourceMgr.getPresumedLoc(Tok.getLocation());
 
 OutBuffer.clear();
 OutBuffer.push_back('"');
 
 if (PLoc.isValid()) {
   TokenBuffer.clear();
   TokenBuffer.append(PLoc.getFilename());
   
   size_t LastSlashPos = TokenBuffer.find_last_of('/');
   if (LastSlashPos != StringRef::npos) {
 std::string LastPathComponent = 
   Lexer::Stringify(TokenBuffer.substr(LastSlashPos+1));
 
 OutBuffer.append(LastPathComponent);
   }
   else {
 Lexer::Stringify(TokenBuffer);
 OutBuffer.append(TokenBuffer.str());
   }
 }
 else {
   OutBuffer.push_back('?');
 }
   }

I think this type of approach (I don't mean the PP extension, just the 
methodology) for getting the filename without the full path or shedding parts 
of the full path is a lot nicer. I'm not entirely sure why so many headers are 
getting pulled in either, though, I think the only issue is the path 
separator?. Aside from that I don't think this needs a compiler flag, but a 
macro for just getting the filename reliably in upstream would be nice. Your 
flag idea means various build systems may have to do gymnastics to find out the 
right path to strip so it seems cumbersome.


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

https://reviews.llvm.org/D17741



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

[PATCH] D56226: [clang-format] square parens that are followed by '=' are not Objective-C message sends

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

I was thinking the same (that we might be able to make the detection more 
precise) but I got the impression these cases are handled later (L524 and 
below).


Repository:
  rC Clang

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

https://reviews.llvm.org/D56226



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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 180073.
MyDeveloperDay marked 14 inline comments as done.
MyDeveloperDay added a comment.

Rebase

- update to address most review comments from @JonasToth
- simplify matcher (with less unless)
- remove the need to look for "type-parameter-" string


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

https://reviews.llvm.org/D55433

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseNodiscardCheck.cpp
  clang-tidy/modernize/UseNodiscardCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-nodiscard.rst
  test/clang-tidy/modernize-use-nodiscard-clang-unused.cpp
  test/clang-tidy/modernize-use-nodiscard-cxx11.cpp
  test/clang-tidy/modernize-use-nodiscard-gcc-unused.cpp
  test/clang-tidy/modernize-use-nodiscard-no-macro-inscope-cxx11.cpp
  test/clang-tidy/modernize-use-nodiscard-no-macro.cpp
  test/clang-tidy/modernize-use-nodiscard.cpp
  test/clang-tidy/modernize-use-nodiscard.h

Index: test/clang-tidy/modernize-use-nodiscard.h
===
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard.h
@@ -0,0 +1,5 @@
+
+#define MUST_USE_RESULT __attribute__((warn_unused_result))
+#define NO_DISCARD [[nodiscard]]
+#define NO_RETURN [[noreturn]]
+
Index: test/clang-tidy/modernize-use-nodiscard.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-nodiscard.cpp
@@ -0,0 +1,210 @@
+// RUN: %check_clang_tidy %s modernize-use-nodiscard %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-nodiscard.ReplacementString, value: 'NO_DISCARD'}]}" \
+// RUN: -- -std=c++17 
+
+#include 
+
+namespace boost {
+template  
+class function;
+}
+
+#include "modernize-use-nodiscard.h"
+
+#define BOOLEAN_FUNC bool f23() const
+
+typedef unsigned my_unsigned;
+typedef unsigned &my_unsigned_reference;
+typedef const unsigned &my_unsigned_const_reference;
+
+class Foo {
+public:
+using size_type = unsigned;
+
+bool f1() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f1' should be marked NO_DISCARD [modernize-use-nodiscard] 
+// CHECK-FIXES: NO_DISCARD bool f1() const;
+
+bool f2(int) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f2' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f2(int) const;
+
+bool f3(const int &) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f3(const int &) const;
+
+bool f4(void) const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f4' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f4(void) const;
+
+// negative tests
+
+void f5() const;
+
+bool f6();
+
+bool f7(int &);
+
+bool f8(int &) const;
+
+bool f9(int *) const;
+
+bool f10(const int &, int &) const;
+
+NO_DISCARD bool f12() const;
+
+MUST_USE_RESULT bool f13() const;
+
+[[nodiscard]] bool f11() const;
+
+[[clang::warn_unused_result]] bool f11a() const;
+
+[[gnu::warn_unused_result]] bool f11b() const;
+
+bool _f20() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function '_f20' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool _f20() const;
+
+NO_RETURN bool f21() const;
+
+~Foo();
+
+bool operator+=(int) const;
+
+// extra keywords (virtual,inline,const) on return type
+
+virtual bool f14() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f14' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD virtual bool f14() const;
+
+const bool f15() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f15' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD const bool f15() const;
+
+inline const bool f16() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f16' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline const bool f16() const;
+
+inline const std::string &f45() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f45' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline const std::string &f45() const;
+
+inline virtual const bool f17() const;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f17' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD inline virtual const bool f17() const;
+
+// inline with body
+bool f18() const {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f18' should be marked NO_DISCARD [modernize-use-nodiscard]
+// CHECK-FIXES: NO_DISCARD bool f18() cons

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:30
+// TODO: Find a better way of detecting a function template.
+static bool isFunctionTemplate(const QualType &ParamType) {
+  // Try to catch both std::function and boost::function

JonasToth wrote:
> Do you want a function-template (`template  void foo(T argument);`) 
> or the template-function `{std,boost}::function`?
> For the first the approach should be the same as above, for the second your 
> implementation might work.
> 
> But a matcher with `hasAnyName("::std::function", "::boost::function")` would 
> be cleaner if you can use that instead.
I can't seem to get the hasName or hasAnyName matcher pick up std::function<> 
as an argument

Using clang-query, I can't get this to work, did I miss something?

```
match 
functionDecl(hasAnyParameter(hasType(namedDecl(hasName("::std::function").bind("x")
```



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

https://reviews.llvm.org/D55433



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

[PATCH] D56271: [SemaCXX] Fix ICE for unexpanded parameter pack

[Brian Gesiak via Phabricator via cfe-commits]

modocache created this revision.
modocache added a reviewer: rsmith.

The documentation for RecursiveASTVisitor::TraverseDecl states that the
Decl being traversed may be null. In fact, this is the case when a
CXXCatchStmt with no exception decl is traversed. Because the visitor
for diagnosing unexpanded parameter packs does not check for null, it
ends up crashing when it attempts to call the Decl::isParameterPack
method on a null Decl pointer.

Add a null check to prevent an ICE, and a test case that would crash
otherwise. Also, because the test requires C++ exceptions and C++14,
change the test parameters for the entire test file. (Alternatively, I
thought about adding a new test file, but went with this approach for my
own convenience.)

Co-authored-by: Andreas Molzer 
Co-authored-by: Mara Bos 


Repository:
  rC Clang

https://reviews.llvm.org/D56271

Files:
  lib/Sema/SemaTemplateVariadic.cpp
  test/SemaCXX/alias-template.cpp


Index: test/SemaCXX/alias-template.cpp
===
--- test/SemaCXX/alias-template.cpp
+++ test/SemaCXX/alias-template.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -std=c++11 %s
+// RUN: %clang_cc1 -verify -std=c++14 -fcxx-exceptions %s
 
 namespace RedeclAliasTypedef {
   template using T = int;
@@ -189,3 +189,7 @@
 
 int g = sfinae_me(); // expected-error{{no matching function for call to 
'sfinae_me'}}
 }
+
+namespace NullExceptionDecl {
+template auto get = []() { try { } catch(...) {}; return I; }; // 
expected-error{{initializer contains unexpanded parameter pack 'I'}}
+}
Index: lib/Sema/SemaTemplateVariadic.cpp
===
--- lib/Sema/SemaTemplateVariadic.cpp
+++ lib/Sema/SemaTemplateVariadic.cpp
@@ -164,7 +164,7 @@
   // A function parameter pack is a pack expansion, so cannot contain
   // an unexpanded parameter pack. Likewise for a template parameter
   // pack that contains any references to other packs.
-  if (D->isParameterPack())
+  if (D && D->isParameterPack())
 return true;
 
   return inherited::TraverseDecl(D);


Index: test/SemaCXX/alias-template.cpp
===
--- test/SemaCXX/alias-template.cpp
+++ test/SemaCXX/alias-template.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -std=c++11 %s
+// RUN: %clang_cc1 -verify -std=c++14 -fcxx-exceptions %s
 
 namespace RedeclAliasTypedef {
   template using T = int;
@@ -189,3 +189,7 @@
 
 int g = sfinae_me(); // expected-error{{no matching function for call to 'sfinae_me'}}
 }
+
+namespace NullExceptionDecl {
+template auto get = []() { try { } catch(...) {}; return I; }; // expected-error{{initializer contains unexpanded parameter pack 'I'}}
+}
Index: lib/Sema/SemaTemplateVariadic.cpp
===
--- lib/Sema/SemaTemplateVariadic.cpp
+++ lib/Sema/SemaTemplateVariadic.cpp
@@ -164,7 +164,7 @@
   // A function parameter pack is a pack expansion, so cannot contain
   // an unexpanded parameter pack. Likewise for a template parameter
   // pack that contains any references to other packs.
-  if (D->isParameterPack())
+  if (D && D->isParameterPack())
 return true;
 
   return inherited::TraverseDecl(D);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner updated this revision to Diff 180075.
ArnaudBienner added a comment.
Herald added a reviewer: serge-sans-paille.

Make -Wstring-plus-int warns even if when the result is not out of bounds


Repository:
  rC Clang

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

https://reviews.llvm.org/D55382

Files:
  bindings/python/tests/cindex/test_diagnostics.py
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/string-plus-int.cpp


Index: test/SemaCXX/string-plus-int.cpp
===
--- test/SemaCXX/string-plus-int.cpp
+++ test/SemaCXX/string-plus-int.cpp
@@ -31,37 +31,36 @@
   consume("foo" + 5);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + index);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + kMyEnum);  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume("foo" + kMySmallEnum); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   consume(5 + "foo");  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(index + "foo");  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(kMyEnum + "foo");  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume(kMySmallEnum + "foo"); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   // FIXME: suggest replacing with "foo"[5]
   consumeChar(*("foo" + 5));  // expected-warning {{adding 'int' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
   consumeChar(*(5 + "foo"));  // expected-warning {{adding 'int' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
 
   consume(L"foo" + 5);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume(L"foo" + 2); // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+
+  consume("foo" + 3);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume("foo" + 4);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume("\pfoo" + 4);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
+
+  #define A "foo"
+  #define B "bar"
+  consume(A B + sizeof(A) - 1); // expected-warning {{to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
 
   // Should not warn.
   consume(&("foo"[3]));
   consume(&("foo"[index]));
   consume(&("foo"[kMyEnum]));
-  consume("foo" + kMySmallEnum);
-  consume(kMySmallEnum + "foo");
 
-  consume(L"foo" + 2);
-
-  consume("foo" + 3);  // Points at the \0
-  consume("foo" + 4);  // Points 1 past the \0, which is legal too.
-  consume("\pfoo" + 4);  // Pascal strings don't have a trailing \0, but they
- // have a leading length byte, so this is fine too.
 
   consume("foo" + kMyOperatorOverloadedEnum);
   consume(kMyOperatorOverloadedEnum + "foo");
-
-  #define A "foo"
-  #define B "bar"
-  consume(A B + sizeof(A) - 1);
 }
 
 template 
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -9143,16 +9143,6 @@
   if (!IsStringPlusInt || IndexExpr->isValueDependent())
 return;
 
-  Expr::EvalResult Result;
-  if (IndexExpr->EvaluateAsInt(Result, Self.getASTContext())) {
-llvm::APSInt index = Result.Val.getInt();
-unsigned StrLenWithNull = StrExpr->getLength() + 1;
-if (index.isNonNegative() &&
-index <= llvm::APSInt(llvm::APInt(index.getBitWidth(), StrLenWithNull),
-  index.isUnsigned()))
-  return;
-  }
-
   SourceRange DiagRange(LHSExpr->getBeginLoc(), RHSExpr->getEndLoc());
   Self.Diag(OpLoc, diag::warn_string_plus_int)
   << DiagRange << IndexExpr->IgnoreImpCasts()->getType();
Index: bindings/python/tests/cindex/test_diagnostics.py
==

[clang-tools-extra] r350324 - [clangd] Bump vscode-clangd v0.0.9

[Haojian Wu via cfe-commits]

Author: hokein
Date: Thu Jan  3 07:28:38 2019
New Revision: 350324

URL: http://llvm.org/viewvc/llvm-project?rev=350324&view=rev
Log:
[clangd] Bump vscode-clangd v0.0.9

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

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=350324&r1=350323&r2=350324&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Thu Jan  
3 07:28:38 2019
@@ -2,7 +2,7 @@
 "name": "vscode-clangd",
 "displayName": "vscode-clangd",
 "description": "Clang Language Server",
-"version": "0.0.8",
+"version": "0.0.9",
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html";,
 "engines": {


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

[clang-tools-extra] r350325 - [clangd] Move helpers from global namespace into anonymous namespace, NFC.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Thu Jan  3 07:36:18 2019
New Revision: 350325

URL: http://llvm.org/viewvc/llvm-project?rev=350325&view=rev
Log:
[clangd] Move helpers from global namespace into anonymous namespace, NFC.

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=350325&r1=350324&r2=350325&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu Jan  3 07:36:18 2019
@@ -1152,28 +1152,6 @@ Optional speculativeFu
   return CachedReq;
 }
 
-} // namespace
-
-clang::CodeCompleteOptions CodeCompleteOptions::getClangCompleteOpts() const {
-  clang::CodeCompleteOptions Result;
-  Result.IncludeCodePatterns = EnableSnippets && IncludeCodePatterns;
-  Result.IncludeMacros = IncludeMacros;
-  Result.IncludeGlobals = true;
-  // We choose to include full comments and not do doxygen parsing in
-  // completion.
-  // FIXME: ideally, we should support doxygen in some form, e.g. do markdown
-  // formatting of the comments.
-  Result.IncludeBriefComments = false;
-
-  // When an is used, Sema is responsible for completing the main file,
-  // the index can provide results from the preamble.
-  // Tell Sema not to deserialize the preamble to look for results.
-  Result.LoadExternal = !Index;
-  Result.IncludeFixIts = IncludeFixIts;
-
-  return Result;
-}
-
 // Returns the most popular include header for \p Sym. If two headers are
 // equally popular, prefer the shorter one. Returns empty string if \p Sym has
 // no include header.
@@ -1584,6 +1562,28 @@ private:
   }
 };
 
+} // namespace
+
+clang::CodeCompleteOptions CodeCompleteOptions::getClangCompleteOpts() const {
+  clang::CodeCompleteOptions Result;
+  Result.IncludeCodePatterns = EnableSnippets && IncludeCodePatterns;
+  Result.IncludeMacros = IncludeMacros;
+  Result.IncludeGlobals = true;
+  // We choose to include full comments and not do doxygen parsing in
+  // completion.
+  // FIXME: ideally, we should support doxygen in some form, e.g. do markdown
+  // formatting of the comments.
+  Result.IncludeBriefComments = false;
+
+  // When an is used, Sema is responsible for completing the main file,
+  // the index can provide results from the preamble.
+  // Tell Sema not to deserialize the preamble to look for results.
+  Result.LoadExternal = !Index;
+  Result.IncludeFixIts = IncludeFixIts;
+
+  return Result;
+}
+
 Expected speculateCompletionFilter(StringRef Content, Position Pos) 
{
   auto Offset = positionToOffset(Content, Pos);
   if (!Offset)


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

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Nolan O'Brien via Phabricator via cfe-commits]

NSProgrammer added a comment.

To throw in my 2 cents.  I don’t really have a preference between a compiler 
flag vs a different macro that’s just for the file name sans path prefix.  But 
I have a real need for this to get into clang:  with 1.2 million lines of code, 
the regular placement of log statements and custom asserts leads to megabytes 
in binary size from all the __FILE__ usages, and that could easily be a few 
hundred KB with this kind of support in clang.


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

https://reviews.llvm.org/D17741



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

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Roman Lebedev via cfe-commits]

Just a thought: reproducible builds will surely be interested in this.
Did gcc already solve this problem? And if yes, it would be best to be
compatible.

On Thu, Jan 3, 2019 at 6:41 PM Nolan O'Brien via Phabricator via
cfe-commits  wrote:
>
> NSProgrammer added a comment.
>
> To throw in my 2 cents.  I don’t really have a preference between a compiler 
> flag vs a different macro that’s just for the file name sans path prefix.  
> But I have a real need for this to get into clang:  with 1.2 million lines of 
> code, the regular placement of log statements and custom asserts leads to 
> megabytes in binary size from all the __FILE__ usages, and that could easily 
> be a few hundred KB with this kind of support in clang.
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D17741/new/
>
> https://reviews.llvm.org/D17741
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Roman Lebedev via cfe-commits]

No, that is the opposite of the solution, actually, i think.
https://tests.reproducible-builds.org/debian/issues/unstable/gcc_captures_build_path_issue.html

And:
https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00513.html
^ i suspect clang should be compatible with whatever approach is
suggested there,
though i have not looked what it is. (cc'd the author of that patch here)

On Thu, Jan 3, 2019 at 6:54 PM Nolan O'Brien  wrote:
>
> I believe reproducible build projects simply move their source to 
> `/tmp/build` or something else that’s consistent to deal with `__FILE__` 
> currently.  They’d probably appreciate this to avoid file copying during 
> `make` builds
>
> Nolan O'Brien
> Sent from my iPhone
>
> > On Jan 3, 2019, at 7:47 AM, Roman Lebedev  wrote:
> >
> > Just a thought: reproducible builds will surely be interested in this.
> > Did gcc already solve this problem? And if yes, it would be best to be
> > compatible.
> >
> > On Thu, Jan 3, 2019 at 6:41 PM Nolan O'Brien via Phabricator via
> > cfe-commits  wrote:
> >>
> >> NSProgrammer added a comment.
> >>
> >> To throw in my 2 cents.  I don’t really have a preference between a 
> >> compiler flag vs a different macro that’s just for the file name sans path 
> >> prefix.  But I have a real need for this to get into clang:  with 1.2 
> >> million lines of code, the regular placement of log statements and custom 
> >> asserts leads to megabytes in binary size from all the __FILE__ usages, 
> >> and that could easily be a few hundred KB with this kind of support in 
> >> clang.
> >>
> >>
> >> CHANGES SINCE LAST ACTION
> >>  https://reviews.llvm.org/D17741/new/
> >>
> >> https://reviews.llvm.org/D17741
> >>
> >>
> >>
> >> ___
> >> cfe-commits mailing list
> >> cfe-commits@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

In D17741#1345171 , @NSProgrammer 
wrote:

> To throw in my 2 cents.  I don’t really have a preference between a compiler 
> flag vs a different macro that’s just for the file name sans path prefix.  
> But I have a real need for this to get into clang:  with 1.2 million lines of 
> code, the regular placement of log statements and custom asserts leads to 
> megabytes in binary size from all the __FILE__ usages, and that could easily 
> be a few hundred KB with this kind of support in clang.


Yeah that would only require adding a new macro in 
`lib/Lex/PPMacroExpansion.cpp` plus a test instead of a huge diff as it is now. 
So yes as I said, that would be a great patch. However this implementation is 
is needlessly complicated and I don't think this should be yet another 
driver/compiler flag for a seemingly obscure use case. So yes I'm with you on 
that, filename without the path prefix is definitely an extension I'd like to 
see upstreamed (I proposed it on `cfe-dev` a while ago but it wasn't well 
received so I never put it up for review (that was before the weird 
preprocessor stuff that I do now in my fork)).


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

https://reviews.llvm.org/D17741



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

[PATCH] D56273: Validate -add-plugin arguments.

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: rnk.

-plugin already prints an error if the name of an unknown plugin is passed. 
-add-plugin used to silently ignore that, now it errors too.


https://reviews.llvm.org/D56273

Files:
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -33,6 +33,7 @@
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/Frontend/LangStandard.h"
 #include "clang/Frontend/MigratorOptions.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
@@ -1663,7 +1664,20 @@
 Opts.ProgramAction = frontend::PluginAction;
 Opts.ActionName = A->getValue();
   }
-  Opts.AddPluginActions = Args.getAllArgValues(OPT_add_plugin);
+  for (const std::string &Arg : Args.getAllArgValues(OPT_add_plugin)) {
+bool Found = false;
+for (FrontendPluginRegistry::iterator it = FrontendPluginRegistry::begin(),
+  ie = FrontendPluginRegistry::end();
+ it != ie; ++it) {
+  if (it->getName() == Arg)
+Found = true;
+}
+if (!Found) {
+  Diags.Report(diag::err_fe_invalid_plugin_name) << Arg;
+  continue;
+}
+Opts.AddPluginActions.push_back(Arg);
+  }
   for (const auto *AA : Args.filtered(OPT_plugin_arg))
 Opts.PluginArgs[AA->getValue(0)].emplace_back(AA->getValue(1));
 


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -33,6 +33,7 @@
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/Frontend/LangStandard.h"
 #include "clang/Frontend/MigratorOptions.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
@@ -1663,7 +1664,20 @@
 Opts.ProgramAction = frontend::PluginAction;
 Opts.ActionName = A->getValue();
   }
-  Opts.AddPluginActions = Args.getAllArgValues(OPT_add_plugin);
+  for (const std::string &Arg : Args.getAllArgValues(OPT_add_plugin)) {
+bool Found = false;
+for (FrontendPluginRegistry::iterator it = FrontendPluginRegistry::begin(),
+  ie = FrontendPluginRegistry::end();
+ it != ie; ++it) {
+  if (it->getName() == Arg)
+Found = true;
+}
+if (!Found) {
+  Diags.Report(diag::err_fe_invalid_plugin_name) << Arg;
+  continue;
+}
+Opts.AddPluginActions.push_back(Arg);
+  }
   for (const auto *AA : Args.filtered(OPT_plugin_arg))
 Opts.PluginArgs[AA->getValue(0)].emplace_back(AA->getValue(1));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56273: Validate -add-plugin arguments.

[Nico Weber via Phabricator via cfe-commits]

thakis updated this revision to Diff 180084.
thakis added a comment.

forgot to add test


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

https://reviews.llvm.org/D56273

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Frontend/plugin-unknown.c


Index: clang/test/Frontend/plugin-unknown.c
===
--- /dev/null
+++ clang/test/Frontend/plugin-unknown.c
@@ -0,0 +1,6 @@
+// RUN: not %clang_cc1 -plugin asdf %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -add-plugin asdf %s 2>&1 | FileCheck --check-prefix=ADD 
%s
+// REQUIRES: plugins
+
+// CHECK: unable to find plugin 'asdf'
+// ADD: unable to find plugin 'asdf'
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -33,6 +33,7 @@
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/Frontend/LangStandard.h"
 #include "clang/Frontend/MigratorOptions.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
@@ -1663,7 +1664,20 @@
 Opts.ProgramAction = frontend::PluginAction;
 Opts.ActionName = A->getValue();
   }
-  Opts.AddPluginActions = Args.getAllArgValues(OPT_add_plugin);
+  for (const std::string &Arg : Args.getAllArgValues(OPT_add_plugin)) {
+bool Found = false;
+for (FrontendPluginRegistry::iterator it = FrontendPluginRegistry::begin(),
+  ie = FrontendPluginRegistry::end();
+ it != ie; ++it) {
+  if (it->getName() == Arg)
+Found = true;
+}
+if (!Found) {
+  Diags.Report(diag::err_fe_invalid_plugin_name) << Arg;
+  continue;
+}
+Opts.AddPluginActions.push_back(Arg);
+  }
   for (const auto *AA : Args.filtered(OPT_plugin_arg))
 Opts.PluginArgs[AA->getValue(0)].emplace_back(AA->getValue(1));
 


Index: clang/test/Frontend/plugin-unknown.c
===
--- /dev/null
+++ clang/test/Frontend/plugin-unknown.c
@@ -0,0 +1,6 @@
+// RUN: not %clang_cc1 -plugin asdf %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -add-plugin asdf %s 2>&1 | FileCheck --check-prefix=ADD %s
+// REQUIRES: plugins
+
+// CHECK: unable to find plugin 'asdf'
+// ADD: unable to find plugin 'asdf'
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -33,6 +33,7 @@
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/Frontend/LangStandard.h"
 #include "clang/Frontend/MigratorOptions.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
@@ -1663,7 +1664,20 @@
 Opts.ProgramAction = frontend::PluginAction;
 Opts.ActionName = A->getValue();
   }
-  Opts.AddPluginActions = Args.getAllArgValues(OPT_add_plugin);
+  for (const std::string &Arg : Args.getAllArgValues(OPT_add_plugin)) {
+bool Found = false;
+for (FrontendPluginRegistry::iterator it = FrontendPluginRegistry::begin(),
+  ie = FrontendPluginRegistry::end();
+ it != ie; ++it) {
+  if (it->getName() == Arg)
+Found = true;
+}
+if (!Found) {
+  Diags.Report(diag::err_fe_invalid_plugin_name) << Arg;
+  continue;
+}
+Opts.AddPluginActions.push_back(Arg);
+  }
   for (const auto *AA : Args.filtered(OPT_plugin_arg))
 Opts.PluginArgs[AA->getValue(0)].emplace_back(AA->getValue(1));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56273: Validate -add-plugin arguments.

[Nico Weber via Phabricator via cfe-commits]

thakis updated this revision to Diff 180085.
thakis added a comment.

remove require


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

https://reviews.llvm.org/D56273

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Frontend/plugin-unknown.c


Index: clang/test/Frontend/plugin-unknown.c
===
--- /dev/null
+++ clang/test/Frontend/plugin-unknown.c
@@ -0,0 +1,5 @@
+// RUN: not %clang_cc1 -plugin asdf %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -add-plugin asdf %s 2>&1 | FileCheck --check-prefix=ADD 
%s
+
+// CHECK: unable to find plugin 'asdf'
+// ADD: unable to find plugin 'asdf'
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -33,6 +33,7 @@
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/Frontend/LangStandard.h"
 #include "clang/Frontend/MigratorOptions.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
@@ -1663,7 +1664,20 @@
 Opts.ProgramAction = frontend::PluginAction;
 Opts.ActionName = A->getValue();
   }
-  Opts.AddPluginActions = Args.getAllArgValues(OPT_add_plugin);
+  for (const std::string &Arg : Args.getAllArgValues(OPT_add_plugin)) {
+bool Found = false;
+for (FrontendPluginRegistry::iterator it = FrontendPluginRegistry::begin(),
+  ie = FrontendPluginRegistry::end();
+ it != ie; ++it) {
+  if (it->getName() == Arg)
+Found = true;
+}
+if (!Found) {
+  Diags.Report(diag::err_fe_invalid_plugin_name) << Arg;
+  continue;
+}
+Opts.AddPluginActions.push_back(Arg);
+  }
   for (const auto *AA : Args.filtered(OPT_plugin_arg))
 Opts.PluginArgs[AA->getValue(0)].emplace_back(AA->getValue(1));
 


Index: clang/test/Frontend/plugin-unknown.c
===
--- /dev/null
+++ clang/test/Frontend/plugin-unknown.c
@@ -0,0 +1,5 @@
+// RUN: not %clang_cc1 -plugin asdf %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -add-plugin asdf %s 2>&1 | FileCheck --check-prefix=ADD %s
+
+// CHECK: unable to find plugin 'asdf'
+// ADD: unable to find plugin 'asdf'
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -33,6 +33,7 @@
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/Frontend/LangStandard.h"
 #include "clang/Frontend/MigratorOptions.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
@@ -1663,7 +1664,20 @@
 Opts.ProgramAction = frontend::PluginAction;
 Opts.ActionName = A->getValue();
   }
-  Opts.AddPluginActions = Args.getAllArgValues(OPT_add_plugin);
+  for (const std::string &Arg : Args.getAllArgValues(OPT_add_plugin)) {
+bool Found = false;
+for (FrontendPluginRegistry::iterator it = FrontendPluginRegistry::begin(),
+  ie = FrontendPluginRegistry::end();
+ it != ie; ++it) {
+  if (it->getName() == Arg)
+Found = true;
+}
+if (!Found) {
+  Diags.Report(diag::err_fe_invalid_plugin_name) << Arg;
+  continue;
+}
+Opts.AddPluginActions.push_back(Arg);
+  }
   for (const auto *AA : Args.filtered(OPT_plugin_arg))
 Opts.PluginArgs[AA->getValue(0)].emplace_back(AA->getValue(1));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56273: Validate -add-plugin arguments.

[Nico Weber via Phabricator via cfe-commits]

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



Comment at: clang/test/Frontend/plugin-unknown.c:3
+// RUN: not %clang_cc1 -add-plugin asdf %s 2>&1 | FileCheck --check-prefix=ADD 
%s
+// REQUIRES: plugins
+

Probably doesn't even require plugins, since we don't need to load any shared 
libaries for this test. I'll remove this.


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

https://reviews.llvm.org/D56273



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

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Nolan O'Brien via cfe-commits]

I believe reproducible build projects simply move their source to `/tmp/build` 
or something else that’s consistent to deal with `__FILE__` currently.  They’d 
probably appreciate this to avoid file copying during `make` builds

Nolan O'Brien
Sent from my iPhone

> On Jan 3, 2019, at 7:47 AM, Roman Lebedev  wrote:
> 
> Just a thought: reproducible builds will surely be interested in this.
> Did gcc already solve this problem? And if yes, it would be best to be
> compatible.
> 
> On Thu, Jan 3, 2019 at 6:41 PM Nolan O'Brien via Phabricator via
> cfe-commits  wrote:
>> 
>> NSProgrammer added a comment.
>> 
>> To throw in my 2 cents.  I don’t really have a preference between a compiler 
>> flag vs a different macro that’s just for the file name sans path prefix.  
>> But I have a real need for this to get into clang:  with 1.2 million lines 
>> of code, the regular placement of log statements and custom asserts leads to 
>> megabytes in binary size from all the __FILE__ usages, and that could easily 
>> be a few hundred KB with this kind of support in clang.
>> 
>> 
>> CHANGES SINCE LAST ACTION
>>  https://reviews.llvm.org/D17741/new/
>> 
>> https://reviews.llvm.org/D17741
>> 
>> 
>> 
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r350328 - [OPENMP][NVPTX]Use __kmpc_barrier_simple_spmd(nullptr, 0) instead of

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Thu Jan  3 08:25:35 2019
New Revision: 350328

URL: http://llvm.org/viewvc/llvm-project?rev=350328&view=rev
Log:
[OPENMP][NVPTX]Use __kmpc_barrier_simple_spmd(nullptr, 0) instead of
nvvm_barrier0.

Use runtime functions instead of the direct call to the nvvm intrinsics.
It allows to prevent some dangerous LLVM optimizations, that breaks the
code for the NVPTX target.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp
cfe/trunk/test/OpenMP/nvptx_parallel_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_parallel_for_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_teams_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=350328&r1=350327&r2=350328&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Thu Jan  3 08:25:35 2019
@@ -96,8 +96,11 @@ enum OpenMPRTLFunctionNVPTX {
   OMPRTL_NVPTX__kmpc_get_team_static_memory,
   /// Call to void __kmpc_restore_team_static_memory(int16_t is_shared);
   OMPRTL_NVPTX__kmpc_restore_team_static_memory,
-  // Call to void __kmpc_barrier(ident_t *loc, kmp_int32 global_tid);
+  /// Call to void __kmpc_barrier(ident_t *loc, kmp_int32 global_tid);
   OMPRTL__kmpc_barrier,
+  /// Call to void __kmpc_barrier_simple_spmd(ident_t *loc, kmp_int32
+  /// global_tid);
+  OMPRTL__kmpc_barrier_simple_spmd,
 };
 
 /// Pre(post)-action for different OpenMP constructs specialized for NVPTX.
@@ -640,17 +643,6 @@ static llvm::Value *getNVPTXNumThreads(C
   "nvptx_num_threads");
 }
 
-/// Get barrier to synchronize all threads in a block.
-static void getNVPTXCTABarrier(CodeGenFunction &CGF) {
-  llvm::Function *F = llvm::Intrinsic::getDeclaration(
-  &CGF.CGM.getModule(), llvm::Intrinsic::nvvm_barrier0);
-  F->addFnAttr(llvm::Attribute::Convergent);
-  CGF.EmitRuntimeCall(F);
-}
-
-/// Synchronize all GPU threads in a block.
-static void syncCTAThreads(CodeGenFunction &CGF) { getNVPTXCTABarrier(CGF); }
-
 /// Get the value of the thread_limit clause in the teams directive.
 /// For the 'generic' execution mode, the runtime encodes thread_limit in
 /// the launch parameters, always starting thread_limit+warpSize threads per
@@ -1813,6 +1805,17 @@ CGOpenMPRuntimeNVPTX::createNVPTXRuntime
 cast(RTLFn)->addFnAttr(llvm::Attribute::Convergent);
 break;
   }
+  case OMPRTL__kmpc_barrier_simple_spmd: {
+// Build void __kmpc_barrier_simple_spmd(ident_t *loc, kmp_int32
+// global_tid);
+llvm::Type *TypeParams[] = {getIdentTyPointerTy(), CGM.Int32Ty};
+auto *FnTy =
+llvm::FunctionType::get(CGM.VoidTy, TypeParams, /*isVarArg*/ false);
+RTLFn =
+CGM.CreateRuntimeFunction(FnTy, /*Name*/ "__kmpc_barrier_simple_spmd");
+cast(RTLFn)->addFnAttr(llvm::Attribute::Convergent);
+break;
+  }
   }
   return RTLFn;
 }
@@ -2665,6 +2668,20 @@ void CGOpenMPRuntimeNVPTX::emitSPMDParal
   }
 }
 
+void CGOpenMPRuntimeNVPTX::syncCTAThreads(CodeGenFunction &CGF) {
+  // Always emit simple barriers!
+  if (!CGF.HaveInsertPoint())
+return;
+  // Build call __kmpc_barrier_simple_spmd(nullptr, 0);
+  // This function does not use parameters, so we can emit just default values.
+  llvm::Value *Args[] = {
+  llvm::ConstantPointerNull::get(
+  cast(getIdentTyPointerTy())),
+  llvm::ConstantInt::get(CGF.Int32Ty, /*V=*/0, /*isSigned=*/true)};
+  CGF.EmitRuntimeCall(
+  createNVPTXRuntimeFunction(OMPRTL__kmpc_barrier_simple_spmd), Args);
+}
+
 void CGOpenMPRuntimeNVPTX::emitBarrierCall(CodeGenFunction &CGF,
SourceLocation Loc,
OpenMPDirectiveKind Kind, bool,

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h?rev=350328&r1=350327&r2=350328&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h Thu Jan  3 08:25:35 2019
@@ -58,6 +58,9 @@ private:
 
   bool requiresFullRuntime() const { return RequiresFullRuntime; }
 
+  /// Get barrier to synchronize all threads in a block.
+  void syncCTAThreads(CodeGenFunction &CGF);
+
   /// Emit the worker function for the current target region.
   void emitWorkerFunction(WorkerFunctionState &WST);
 

Modified: cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp?rev=350328&r1=350327&r2=350328&view=diff
=

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[serge via Phabricator via cfe-commits]

serge-sans-paille resigned from this revision.
serge-sans-paille added a comment.

Just wanted to review the change to the Python script to ensure it remains 
compatible with Python2 and Python3, which is indeed the case; so LGTM on that 
aspect.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55382



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D56113#1345047 , @ABataev wrote:

> In D56113#1344210 , @jdenny wrote:
>
> > In D56113#1342879 , @ABataev wrote:
> >
> > > But you will need another serie of patches for `reduction` and `linear` 
> > > clauses to update their error messages.
> >
> >
> > Those already had their own checks for const.  I'm not aware of any cases 
> > not handled, and the test suite does pass after my patch.
>
>
> Yes, they have the checks for constness, but you need to  update those checks 
> to emit this new error message for const values with mutable fields.


I believe you mean we should reuse `rejectConstNotMutableType` for `reduction` 
and `linear` rather than leave some of its implementation duplicated for those.

That is, I believe we should //not// relax the existing checks for `reduction` 
and `linear` so that they permit const values with mutable fields.  First, that 
doesn't seem possible for `linear`, which requires integral or pointer type.  
Second, that seems wrong for `reduction` because, for whatever reason, OpenMP 
5.0 sec. 2.19.5.1 page 298 says:

  A list item that appears in a reduction clause must not be const-qualified.

For that case, I can extend `rejectConstNotMutableType` with a bool parameter 
to indicate mutable fields are not permitted.

Does all that make sense?

> 
> 
>> By the way, is there any value to keeping the predetermined shared for const 
>> if -openmp-version=3.1 or earlier?
> 
> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 it will 
> have the value `31`.

How far back should we take this?  I'm inclined to check for `30` and `31` only 
and assume anything else is newer, but let me know if we need to check for 
earlier versions.

> 
> 
 Sorry, I misunderstood your first response.  When I try an atomic write to 
 the shared variable, the difference between the uncombined and combined 
 target teams directive affects functionality when targeting multicore.  
 Does that matter?
>>> 
>>> Yes, I'm aware if this problem. But I don't think it is very important and 
>>> can cause serious problems in user code. If you think that it is worth to 
>>> fix this prblem, you can go ahead and fix it.
>> 
>> No, not worthwhile for me right now.  I just wanted to point it out in 
>> passing in case it might matter to someone.  Does a bugzilla seem worthwhile 
>> to you?
> 
> Yes, generally speaking it would be good to fix this. So, yes, at least to 
> keep tracking, add a bug report to the bugzilla.

Will do.


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

https://reviews.llvm.org/D56113



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

[PATCH] D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaDeclCXX.cpp:8175
   DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo();
-  if (FTI.TypeQuals != 0) {
-if (FTI.TypeQuals & Qualifiers::Const)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "const" << SourceRange(D.getIdentifierLoc());
-if (FTI.TypeQuals & Qualifiers::Volatile)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "volatile" << SourceRange(D.getIdentifierLoc());
-if (FTI.TypeQuals & Qualifiers::Restrict)
-  Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
-<< "restrict" << SourceRange(D.getIdentifierLoc());
+  if (FTI.MethodQualifiers && FTI.MethodQualifiers->getTypeQualifiers() != 0) {
+auto DiagQual = [&](DeclSpec::TQ TypeQual, StringRef QualName,

Anastasia wrote:
> rjmccall wrote:
> > Anastasia wrote:
> > > rjmccall wrote:
> > > > I think you should add a `hasMethodQualifiers` method to FTI that does 
> > > > this check.  Note that it needs to check for attributes, too, and I 
> > > > think you need to figure out some way to generalize `forEachCVRUQual` 
> > > > to cover those.
> > > Are there any attributes I should handle currently?
> > > 
> > > Also are you suggesting to add another `forEach...` method or extend 
> > > existing? If the latter, I might not be able to use it in all places I 
> > > use it now.
> > Adding another method might be easier.  How many clients actually use the 
> > TQ?
> In **DeclSpec.cpp** I definitely  need just TQ. I am not sure about 
> **SemaType.cpp**. All other places (3x) I guess should be possible to 
> generalize. Although I am not very clear if I should be checking all attr. It 
> might be a bit exhaustive since the use cases are for the function?
> 
> Perhaps, I could add an extra helper `forEachQualifier` that can call 
> `forEachCVRUQual` and then I could add a FIXME to complete the rest. We can 
> extend it as we discover what's missing. For example I will add address 
> spaces there in my next patch. Would this make sense?
> 
> As for `hasMethodQualifiers` just to be clear I would need to check for all 
> qualifiers including reference qualifier, attributes, etc?
That seems like a reasonable short-term plan.  Maybe there needs to be some way 
to describe an individual qualifier; we can hash that out in a separate patch.

> As for `hasMethodQualifiers` just to be clear I would need to check for all 
> qualifiers including reference qualifier, attributes, etc?

Maybe, although at least one of the cases below wants to check for 
ref-qualifiers separately.  Maybe it should be `hasMethodTypeQualifiers`, and 
it implies that `MethodQualifiers->forEachQualifier` will invoke the callback 
at least once.


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

https://reviews.llvm.org/D55948



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D56113#1345232 , @jdenny wrote:

> In D56113#1345047 , @ABataev wrote:
>
> > In D56113#1344210 , @jdenny wrote:
> >
> > > In D56113#1342879 , @ABataev 
> > > wrote:
> > >
> > > > But you will need another serie of patches for `reduction` and `linear` 
> > > > clauses to update their error messages.
> > >
> > >
> > > Those already had their own checks for const.  I'm not aware of any cases 
> > > not handled, and the test suite does pass after my patch.
> >
> >
> > Yes, they have the checks for constness, but you need to  update those 
> > checks to emit this new error message for const values with mutable fields.
>
>
> I believe you mean we should reuse `rejectConstNotMutableType` for 
> `reduction` and `linear` rather than leave some of its implementation 
> duplicated for those.
>
> That is, I believe we should //not// relax the existing checks for 
> `reduction` and `linear` so that they permit const values with mutable 
> fields.  First, that doesn't seem possible for `linear`, which requires 
> integral or pointer type.  Second, that seems wrong for `reduction` because, 
> for whatever reason, OpenMP 5.0 sec. 2.19.5.1 page 298 says:
>
>   A list item that appears in a reduction clause must not be const-qualified.
>
>
> For that case, I can extend `rejectConstNotMutableType` with a bool parameter 
> to indicate mutable fields are not permitted.
>
> Does all that make sense?


Yes, sounds good.

> 
> 
>> 
>> 
>>> By the way, is there any value to keeping the predetermined shared for 
>>> const if -openmp-version=3.1 or earlier?
>> 
>> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 it 
>> will have the value `31`.
> 
> How far back should we take this?  I'm inclined to check for `30` and `31` 
> only and assume anything else is newer, but let me know if we need to check 
> for earlier versions.

I think `<= 31` is good. Clang always supported only OpenMP 3.1 and higher.

> 
> 
>> 
>> 
> Sorry, I misunderstood your first response.  When I try an atomic write 
> to the shared variable, the difference between the uncombined and 
> combined target teams directive affects functionality when targeting 
> multicore.  Does that matter?
 
 Yes, I'm aware if this problem. But I don't think it is very important and 
 can cause serious problems in user code. If you think that it is worth to 
 fix this prblem, you can go ahead and fix it.
>>> 
>>> No, not worthwhile for me right now.  I just wanted to point it out in 
>>> passing in case it might matter to someone.  Does a bugzilla seem 
>>> worthwhile to you?
>> 
>> Yes, generally speaking it would be good to fix this. So, yes, at least to 
>> keep tracking, add a bug report to the bugzilla.
> 
> Will do.




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

https://reviews.llvm.org/D56113



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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:30
+// TODO: Find a better way of detecting a function template.
+static bool isFunctionTemplate(const QualType &ParamType) {
+  // Try to catch both std::function and boost::function

MyDeveloperDay wrote:
> JonasToth wrote:
> > Do you want a function-template (`template  void foo(T 
> > argument);`) or the template-function `{std,boost}::function`?
> > For the first the approach should be the same as above, for the second your 
> > implementation might work.
> > 
> > But a matcher with `hasAnyName("::std::function", "::boost::function")` 
> > would be cleaner if you can use that instead.
> I can't seem to get the hasName or hasAnyName matcher pick up std::function<> 
> as an argument
> 
> Using clang-query, I can't get this to work, did I miss something?
> 
> ```
> match 
> functionDecl(hasAnyParameter(hasType(namedDecl(hasName("::std::function").bind("x")
> ```
> 
I experimented a bit and found something that might work for you. See the 
attachment.
{F7791969}


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

https://reviews.llvm.org/D55433



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

[clang-tools-extra] r350329 - [clangd] Fix detecting atomics in stand-alone builds

[Michal Gorny via cfe-commits]

Author: mgorny
Date: Thu Jan  3 08:43:27 2019
New Revision: 350329

URL: http://llvm.org/viewvc/llvm-project?rev=350329&view=rev
Log:
[clangd] Fix detecting atomics in stand-alone builds

Include CheckAtomic CMake module from LLVM in order to detect support
for atomics when building stand-alone.  Otherwise,
the HAVE_CXX_ATOMICS64_WITHOUT_LIB variable is undefined and clangd
wrongly attempts to link -latomic on systems not using the library.

Original bug report: https://bugs.gentoo.org/667016

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

Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=350329&r1=350328&r2=350329&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Thu Jan  3 08:43:27 2019
@@ -2,6 +2,11 @@ set(LLVM_LINK_COMPONENTS
   Support
   )
 
+if(CLANG_BUILT_STANDALONE)
+  # needed to get HAVE_CXX_ATOMICS64_WITHOUT_LIB defined
+  include(CheckAtomic)
+endif()
+
 set(CLANGD_ATOMIC_LIB "")
 if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
   list(APPEND CLANGD_ATOMIC_LIB "atomic")


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

[PATCH] D56061: [clang-tools-extra] [clangd] Fix detecting atomics in stand-alone builds

[Michał Górny via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE350329: [clangd] Fix detecting atomics in stand-alone 
builds (authored by mgorny, committed by ).

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56061

Files:
  clangd/CMakeLists.txt


Index: clangd/CMakeLists.txt
===
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -2,6 +2,11 @@
   Support
   )
 
+if(CLANG_BUILT_STANDALONE)
+  # needed to get HAVE_CXX_ATOMICS64_WITHOUT_LIB defined
+  include(CheckAtomic)
+endif()
+
 set(CLANGD_ATOMIC_LIB "")
 if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
   list(APPEND CLANGD_ATOMIC_LIB "atomic")


Index: clangd/CMakeLists.txt
===
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -2,6 +2,11 @@
   Support
   )
 
+if(CLANG_BUILT_STANDALONE)
+  # needed to get HAVE_CXX_ATOMICS64_WITHOUT_LIB defined
+  include(CheckAtomic)
+endif()
+
 set(CLANGD_ATOMIC_LIB "")
 if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
   list(APPEND CLANGD_ATOMIC_LIB "atomic")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47817: [compiler-rt] [sanitizer_common] Remove support for tirpc/rpc/xdr.h

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

@ygribov, ping.


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

https://reviews.llvm.org/D47817



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

[PATCH] D55933: [Sema] Do not print default template parameters.

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

Quuxplusone added inline comments.



Comment at: lib/AST/TypePrinter.cpp:179
+// `X` gets canonicalized to `X`. We disable
+// canonicalization so that `printTag()`` can see the template parameters 
as
+// written.

Nit: there's an extra ` on this line.

I think I vaguely understand the purpose of this switch now. It feels ugly, but 
I have no concrete suggestion for improvement. Do you not need a case here to 
delay canonicalization of `X&` also?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55933



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

[PATCH] D55932: [Sema] Simplfy static_assert diagnostic code.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/SemaCXX/static-assert.cpp:130
 static_assert(std::is_same()), 
int>::value, "message");
-// expected-error@-1{{static_assert failed due to requirement 
'std::is_same, int>::value' "message"}}
+// expected-error@-1{{static_assert failed due to requirement 
'std::is_same, int>::value' "message"}}
 static_assert(std::is_const::value, "message");

courbet wrote:
> aaron.ballman wrote:
> > courbet wrote:
> > > aaron.ballman wrote:
> > > > courbet wrote:
> > > > > aaron.ballman wrote:
> > > > > > Any idea why the `std::` was dropped here?
> > > > > `NestedNameSpecifier::print()` explicitly does:
> > > > > 
> > > > > ```
> > > > >  PrintingPolicy InnerPolicy(Policy);
> > > > >  InnerPolicy.SuppressScope = true;
> > > > > ```
> > > > > 
> > > > Ah, good point, but is that a good behavioral change? I slightly prefer 
> > > > printing the namespace name there -- it will likely be redundant 
> > > > information most of the time, but when the namespace actually matters, 
> > > > having it printed could save someone a lot of head scratching.
> > > > I slightly prefer printing the namespace name there
> > > 
> > > I tend to agree, so it's more a trade-off of code complexity vs better 
> > > diagnostic - I tend to err on the side of simplifying the code :)
> > > 
> > > Another option is to add yet another boolean to PrintingPolicy, but I 
> > > htink this is too narrow a use case.
> > Heh, I tend to err on the side of helping the user unless the code will be 
> > truly awful. I agree that another option on PrintingPolicy may not be the 
> > best approach. Do you know why the namespace is being suppressed in the 
> > first place? Another option would be to always print the namespace, but I 
> > don't know what that might regress (if anything).
> Unfortunately always printing the qualification breaks 30 tests, including 
> some in a very bad way:
> 
> ```
> /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/AST/ast-print-out-of-line-func.cpp:29:11:
>  error: CHECK: expected string not found in input
> // CHECK: void Wrapper::Inner::operator+=(int)
>   ^
> :14:43: note: scanning from here
>  ns::Wrapper::ns::Wrapper::Inner::Inner() {
>   ^
> :16:19: note: possible intended match here
>  void ns::Wrapper::ns::Wrapper::Inner::operator+=(int) {
> ```
That looks like a bug with the printer, no?

As it stands, I think this is a regression with the diagnostic printing. I 
think it would be reasonable if the namespace were dropped only when the 
namespace reduces clarity (e.g., there are no other conflicting names in other 
namespaces, so the namespace is superfluous), but I don't think that's what's 
happening here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55932



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

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Ximin Luo via cfe-commits]

Hi all, more strict interpretations of reproducible builds want build paths to 
not appear in the final output *regardless of the build location* and so 
/tmp/build would not be appropriate. Rough justification is that we want 
different people building under different starting conditions to be able 
reproduce identical final binaries, this is a *stronger* property than 
reproduction under *same* starting conditions.

I believe newer versions of GCC have an existing flag called -fmacro-prefix-map:

https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html

My patch that Roman quoted in the email below was an additional mechanism to 
set -fmacro-prefix-map and -fdebug-prefix-map via an environment variable. This 
was not accepted into GCC because they are (in summary) allergic against using 
environment variables to set anything.

I agree envvars are not great in general, but think an exception in this case 
was appropriate. It is very common for higher-level build scripts to store 
command-line options in various parts of the build output (making it again 
dependent on the build path, since the argument to -fmacro-prefix-map contains 
the build path). By contrast, it is almost unheard-of for tools to store 
arbitrary environment variables in the build output, and that usually indicates 
a bug.

This is why we the reproducible builds project defined some standardised 
environment variables for these purposes:

https://reproducible-builds.org/specs/source-date-epoch/
https://reproducible-builds.org/specs/build-path-prefix-map/

Inconsistently, GCC accepted our (The Reproducible Builds Project's) patch for 
the former but not the latter, due to different reviewers. (FWIW, the reviewer 
that rejected the latter patch stated that he would not have accepted the 
former patch.)

Ed Maste (added to CC) had previously tried to get SOURCE_DATE_EPOCH accepted 
into LLVM but the effort seems to have stalled.

https://reviews.llvm.org/D20791
https://reviews.llvm.org/D23934

I'll leave it to you guys to decide where to go from here.

X

Roman Lebedev:
> No, that is the opposite of the solution, actually, i think.
> https://tests.reproducible-builds.org/debian/issues/unstable/gcc_captures_build_path_issue.html
> 
> And:
> https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00513.html
> ^ i suspect clang should be compatible with whatever approach is
> suggested there,
> though i have not looked what it is. (cc'd the author of that patch here)
> 
> On Thu, Jan 3, 2019 at 6:54 PM Nolan O'Brien  wrote:
>>
>> I believe reproducible build projects simply move their source to 
>> `/tmp/build` or something else that’s consistent to deal with `__FILE__` 
>> currently.  They’d probably appreciate this to avoid file copying during 
>> `make` builds
>>
>> Nolan O'Brien
>> Sent from my iPhone
>>
>>> On Jan 3, 2019, at 7:47 AM, Roman Lebedev  wrote:
>>>
>>> Just a thought: reproducible builds will surely be interested in this.
>>> Did gcc already solve this problem? And if yes, it would be best to be
>>> compatible.
>>>
>>> On Thu, Jan 3, 2019 at 6:41 PM Nolan O'Brien via Phabricator via
>>> cfe-commits  wrote:

 NSProgrammer added a comment.

 To throw in my 2 cents.  I don’t really have a preference between a 
 compiler flag vs a different macro that’s just for the file name sans path 
 prefix.  But I have a real need for this to get into clang:  with 1.2 
 million lines of code, the regular placement of log statements and custom 
 asserts leads to megabytes in binary size from all the __FILE__ usages, 
 and that could easily be a few hundred KB with this kind of support in 
 clang.


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

 https://reviews.llvm.org/D17741



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


-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56090: Add a matcher for members of an initializer list expression

[Hyrum Wright via Phabricator via cfe-commits]

hwright updated this revision to Diff 180089.
hwright marked an inline comment as done.
hwright added a comment.

Added tests, update docs.


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

https://reviews.llvm.org/D56090

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp


Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2255,6 +2255,18 @@
   notMatches("void x() { int a; if(a == 0) return; }", BinAsgmtOperator));
 }
 
+TEST(HasInit, Basic) {
+  EXPECT_TRUE(
+matches("int x{0};",
+initListExpr(hasInit(0, expr();
+  EXPECT_FALSE(
+matches("int x{0};",
+initListExpr(hasInit(1, expr();
+  EXPECT_FALSE(
+matches("int x;",
+initListExpr(hasInit(0, expr();
+}
+
 TEST(Matcher, isMain) {
   EXPECT_TRUE(
 matches("int main() {}", functionDecl(isMain(;
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -273,6 +273,7 @@
   REGISTER_MATCHER(hasInClassInitializer);
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);
+  REGISTER_MATCHER(hasInit);
   REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -3514,6 +3514,20 @@
   *Node.getArg(N)->IgnoreParenImpCasts(), Finder, Builder));
 }
 
+/// Matches the n'th item of an initializer list expression.
+///
+/// Example matches y.
+/// (matcher = initListExpr(hasInit(0, expr(
+/// \code
+///   int x{y}.
+/// \endcode
+AST_MATCHER_P2(InitListExpr, hasInit, unsigned, N,
+   ast_matchers::internal::Matcher, InnerMatcher) {
+  return N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,
+   Builder);
+}
+
 /// Matches declaration statements that contain a specific number of
 /// declarations.
 ///
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -718,7 +718,7 @@
 
 
 MatcherStmt>blockExprMatcherBlockExpr>...
-MAtches a reference to a 
block.
+Matches a reference to a 
block.
 
 Example: matches "^{}":
   void f() { ^{}(); }
@@ -5866,6 +5866,15 @@
 
 
 
+MatcherInitListExpr>hasInitunsigned N, 
ast_matchers::MatcherExpr> 
InnerMatcher
+Matches the n'th item of an 
initializer list expression.
+
+Example matches y.
+(matcher = initListExpr(hasInit(0, expr(
+  int x{y}.
+
+
+
 MatcherInitListExpr>hasSyntacticFormMatcherExpr> 
InnerMatcher
 Matches the 
syntactic form of init list expressions
 (if expression have it).


Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2255,6 +2255,18 @@
   notMatches("void x() { int a; if(a == 0) return; }", BinAsgmtOperator));
 }
 
+TEST(HasInit, Basic) {
+  EXPECT_TRUE(
+matches("int x{0};",
+initListExpr(hasInit(0, expr();
+  EXPECT_FALSE(
+matches("int x{0};",
+initListExpr(hasInit(1, expr();
+  EXPECT_FALSE(
+matches("int x;",
+initListExpr(hasInit(0, expr();
+}
+
 TEST(Matcher, isMain) {
   EXPECT_TRUE(
 matches("int main() {}", functionDecl(isMain(;
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -273,6 +273,7 @@
   REGISTER_MATCHER(hasInClassInitializer);
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);
+  REGISTER_MATCHER(hasInit);
   REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+

[PATCH] D56090: Add a matcher for members of an initializer list expression

[Hyrum Wright via Phabricator via cfe-commits]

hwright added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:3527
+  return N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,
+   Builder);

aaron.ballman wrote:
> I'm not certain we want the `IgnoreParenImpCasts()` here -- what if someone 
> wants to match an initializer that uses one of those properties?
The `hasArg` implementation directly above has the same call to 
`IgnoreParenImpCasts()`.  Is it also in error?  (It would seem that they should 
both be consistent.)


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

https://reviews.llvm.org/D56090



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

[PATCH] D56090: Add a matcher for members of an initializer list expression

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: klimek.
aaron.ballman added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:3527
+  return N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,
+   Builder);

hwright wrote:
> aaron.ballman wrote:
> > I'm not certain we want the `IgnoreParenImpCasts()` here -- what if someone 
> > wants to match an initializer that uses one of those properties?
> The `hasArg` implementation directly above has the same call to 
> `IgnoreParenImpCasts()`.  Is it also in error?  (It would seem that they 
> should both be consistent.)
Yeah, I noticed that as well. Doing some code archaeology, it seems that 
`hasArg()` has had that form since its inception and it was never explicitly 
discussed what should happen there.

@klimek -- do you have opinions here, since you wrote the original code for 
`hasArg()`? Should we be leaving the paren and implicit cast stripping up to 
the caller rather than doing it automatically?


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

https://reviews.llvm.org/D56090



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

[PATCH] D56090: Add a matcher for members of an initializer list expression

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: klimek.
aaron.ballman added a comment.

Adding Manuel for the `hasArg()` questions.


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

https://reviews.llvm.org/D56090



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/Sema/pragma-pipeline.cpp:3
+
+#pragma clang loop pipeline(disable) /* expected-error {{expected 
unqualified-id}} */
+int main() {

I think this error is pretty unfortunate -- it doesn't really help the user to 
understand what's wrong with their code. Can it be improved?


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

https://reviews.llvm.org/D55710



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

r350335 - Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via cfe-commits]

Author: abien
Date: Thu Jan  3 09:45:28 2019
New Revision: 350335

URL: http://llvm.org/viewvc/llvm-project?rev=350335&view=rev
Log:
Make -Wstring-plus-int warns even if when the result is not out of bounds

Summary: Patch by Arnaud Bienner

Reviewers: sylvestre.ledru, thakis, serge-sans-paille

Reviewed By: thakis

Subscribers: arphaman, dyung, anemet, llvm-commits, cfe-commits

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

Modified:
cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/SemaCXX/string-plus-int.cpp

Modified: cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py?rev=350335&r1=350334&r2=350335&view=diff
==
--- cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py (original)
+++ cfe/trunk/bindings/python/tests/cindex/test_diagnostics.py Thu Jan  3 
09:45:28 2019
@@ -51,7 +51,7 @@ class TestDiagnostics(unittest.TestCase)
 self.assertEqual(tu.diagnostics[0].fixits[0].value, '.f0 = ')
 
 def test_diagnostic_range(self):
-tu = get_tu('void f() { int i = "a" + 1; }')
+tu = get_tu('void f() { int i = "a"; }')
 self.assertEqual(len(tu.diagnostics), 1)
 self.assertEqual(tu.diagnostics[0].severity, Diagnostic.Warning)
 self.assertEqual(tu.diagnostics[0].location.line, 1)
@@ -63,7 +63,7 @@ class TestDiagnostics(unittest.TestCase)
 self.assertEqual(tu.diagnostics[0].ranges[0].start.line, 1)
 self.assertEqual(tu.diagnostics[0].ranges[0].start.column, 20)
 self.assertEqual(tu.diagnostics[0].ranges[0].end.line, 1)
-self.assertEqual(tu.diagnostics[0].ranges[0].end.column, 27)
+self.assertEqual(tu.diagnostics[0].ranges[0].end.column, 23)
 with self.assertRaises(IndexError):
 tu.diagnostics[0].ranges[1].start.line
 

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=350335&r1=350334&r2=350335&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Jan  3 09:45:28 2019
@@ -9143,16 +9143,6 @@ static void diagnoseStringPlusInt(Sema &
   if (!IsStringPlusInt || IndexExpr->isValueDependent())
 return;
 
-  Expr::EvalResult Result;
-  if (IndexExpr->EvaluateAsInt(Result, Self.getASTContext())) {
-llvm::APSInt index = Result.Val.getInt();
-unsigned StrLenWithNull = StrExpr->getLength() + 1;
-if (index.isNonNegative() &&
-index <= llvm::APSInt(llvm::APInt(index.getBitWidth(), StrLenWithNull),
-  index.isUnsigned()))
-  return;
-  }
-
   SourceRange DiagRange(LHSExpr->getBeginLoc(), RHSExpr->getEndLoc());
   Self.Diag(OpLoc, diag::warn_string_plus_int)
   << DiagRange << IndexExpr->IgnoreImpCasts()->getType();

Modified: cfe/trunk/test/SemaCXX/string-plus-int.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/string-plus-int.cpp?rev=350335&r1=350334&r2=350335&view=diff
==
--- cfe/trunk/test/SemaCXX/string-plus-int.cpp (original)
+++ cfe/trunk/test/SemaCXX/string-plus-int.cpp Thu Jan  3 09:45:28 2019
@@ -31,37 +31,36 @@ void f(int index) {
   consume("foo" + 5);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + index);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + kMyEnum);  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume("foo" + kMySmallEnum); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   consume(5 + "foo");  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(index + "foo");  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(kMyEnum + "foo");  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume(kMySmallEnum + "foo"); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   // FIXME: suggest replacing with "foo"[5]
   consumeChar(*("foo" + 5));  // expected-warning {{adding 'int' to a stri

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC350335: Make -Wstring-plus-int warns even if when the result 
is not out of bounds (authored by abien, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55382?vs=180075&id=180093#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55382

Files:
  bindings/python/tests/cindex/test_diagnostics.py
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/string-plus-int.cpp


Index: bindings/python/tests/cindex/test_diagnostics.py
===
--- bindings/python/tests/cindex/test_diagnostics.py
+++ bindings/python/tests/cindex/test_diagnostics.py
@@ -51,7 +51,7 @@
 self.assertEqual(tu.diagnostics[0].fixits[0].value, '.f0 = ')
 
 def test_diagnostic_range(self):
-tu = get_tu('void f() { int i = "a" + 1; }')
+tu = get_tu('void f() { int i = "a"; }')
 self.assertEqual(len(tu.diagnostics), 1)
 self.assertEqual(tu.diagnostics[0].severity, Diagnostic.Warning)
 self.assertEqual(tu.diagnostics[0].location.line, 1)
@@ -63,7 +63,7 @@
 self.assertEqual(tu.diagnostics[0].ranges[0].start.line, 1)
 self.assertEqual(tu.diagnostics[0].ranges[0].start.column, 20)
 self.assertEqual(tu.diagnostics[0].ranges[0].end.line, 1)
-self.assertEqual(tu.diagnostics[0].ranges[0].end.column, 27)
+self.assertEqual(tu.diagnostics[0].ranges[0].end.column, 23)
 with self.assertRaises(IndexError):
 tu.diagnostics[0].ranges[1].start.line
 
Index: test/SemaCXX/string-plus-int.cpp
===
--- test/SemaCXX/string-plus-int.cpp
+++ test/SemaCXX/string-plus-int.cpp
@@ -31,37 +31,36 @@
   consume("foo" + 5);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + index);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + kMyEnum);  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume("foo" + kMySmallEnum); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   consume(5 + "foo");  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(index + "foo");  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(kMyEnum + "foo");  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume(kMySmallEnum + "foo"); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   // FIXME: suggest replacing with "foo"[5]
   consumeChar(*("foo" + 5));  // expected-warning {{adding 'int' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
   consumeChar(*(5 + "foo"));  // expected-warning {{adding 'int' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
 
   consume(L"foo" + 5);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume(L"foo" + 2); // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+
+  consume("foo" + 3);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume("foo" + 4);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume("\pfoo" + 4);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
+
+  #define A "foo"
+  #define B "bar"
+  consume(A B + sizeof(A) - 1); // expected-warning {{to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
 
   // Should not warn.
   consume(&("foo"[3]));
   consume(&("foo"[index]));
   consume(&("foo"[kMyEnum]));
-  consume("foo" + kMySmallEnum);
-  consume(kMySmallEnum + "foo");
 
-  consume(L"foo" + 2);
-
-  consume("foo" + 3);  // Points at the \0
-  consume("foo" + 4);  // Points 1 past the \0, which is legal too.
-  consume(

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

OK, thanks Serge! :)

For the record, I updated the patch one last time after it was accepted to 
remove my change to constant-expression-cxx1y.cpp since someone else did the 
same change in a separate commit meanwhile.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55382



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

In D56215#1344279 , @krytarowski wrote:

> In D56215#1344233 , @ruiu wrote:
>
> > lld's driver is intentionally designed to be agnostic of the host that the 
> > linker is running for the reasons described at the beginning of Driver.cpp: 
> > https://github.com/llvm-project/lld/blob/master/ELF/Driver.cpp#L13 I think 
> > I like that approach. If you need to do this, you can do this in the 
> > compiler driver rather than in the linker itself. Is there any reason you 
> > need to do this in the linker?
>
>
> This breaks compat with GNU ld here and the linker is intended to be used 
> standalone.


This is where lld is not 100% compatible with GNU ld, but I'd think that's not 
a bad thing. I'd like to make lld agnostics of host OS so that the linker works 
exactly in the same way on any operating systems, which makes cross linking 
much easier to do. So, both a run-time detection of a host OS or a 
configure-time customization are I think undesirable.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

[PATCH] D56273: Validate -add-plugin arguments.

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm!


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

https://reviews.llvm.org/D56273



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D56215#1345326 , @ruiu wrote:

> In D56215#1344279 , @krytarowski 
> wrote:
>
> > In D56215#1344233 , @ruiu wrote:
> >
> > > lld's driver is intentionally designed to be agnostic of the host that 
> > > the linker is running for the reasons described at the beginning of 
> > > Driver.cpp: 
> > > https://github.com/llvm-project/lld/blob/master/ELF/Driver.cpp#L13 I 
> > > think I like that approach. If you need to do this, you can do this in 
> > > the compiler driver rather than in the linker itself. Is there any reason 
> > > you need to do this in the linker?
> >
> >
> > This breaks compat with GNU ld here and the linker is intended to be used 
> > standalone.
>
>
> This is where lld is not 100% compatible with GNU ld, but I'd think that's 
> not a bad thing. I'd like to make lld agnostics of host OS so that the linker 
> works exactly in the same way on any operating systems, which makes cross 
> linking much easier to do. So, both a run-time detection of a host OS or a 
> configure-time customization are I think undesirable.


Personally I have no strong opinion either way. Joerg is blocking a patch to 
handle it in clang: https://reviews.llvm.org/D33726

I find it very silly reason to brick NetBSD support in lld. I expect that it's 
easier to push paths into lld than convince @joerg to accept clang patch.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D56113#1345238 , @ABataev wrote:

> >>> By the way, is there any value to keeping the predetermined shared for 
> >>> const if -openmp-version=3.1 or earlier?
> >> 
> >> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 it 
> >> will have the value `31`.
> > 
> > How far back should we take this?  I'm inclined to check for `30` and `31` 
> > only and assume anything else is newer, but let me know if we need to check 
> > for earlier versions.
>
> I think `<= 31` is good. Clang always supported only OpenMP 3.1 and higher.


Ah, for some reason I assumed it was a string not an integer.

In D56113#1345238 , @ABataev wrote:

> In D56113#1345232 , @jdenny wrote:
>
> > In D56113#1345047 , @ABataev wrote:
> >
> > > In D56113#1344210 , @jdenny 
> > > wrote:
> > >
> > > > In D56113#1342879 , @ABataev 
> > > > wrote:
> > > >
> > > > > But you will need another serie of patches for `reduction` and 
> > > > > `linear` clauses to update their error messages.
> > > >
> > > >
> > > > Those already had their own checks for const.  I'm not aware of any 
> > > > cases not handled, and the test suite does pass after my patch.
> > >
> > >
> > > Yes, they have the checks for constness, but you need to  update those 
> > > checks to emit this new error message for const values with mutable 
> > > fields.
> >
> >
> > I believe you mean we should reuse `rejectConstNotMutableType` for 
> > `reduction` and `linear` rather than leave some of its implementation 
> > duplicated for those.
>


For reductions, this will change the message from "const-qualified list item 
cannot be reduction" to "const-qualified variable cannot be reduction".  Is 
that OK?  (There are many tests to update, so I want to ask first.)


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

https://reviews.llvm.org/D56113



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D56113#1345336 , @jdenny wrote:

> In D56113#1345238 , @ABataev wrote:
>
> > >>> By the way, is there any value to keeping the predetermined shared for 
> > >>> const if -openmp-version=3.1 or earlier?
> > >> 
> > >> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 it 
> > >> will have the value `31`.
> > > 
> > > How far back should we take this?  I'm inclined to check for `30` and 
> > > `31` only and assume anything else is newer, but let me know if we need 
> > > to check for earlier versions.
> >
> > I think `<= 31` is good. Clang always supported only OpenMP 3.1 and higher.
>
>
> Ah, for some reason I assumed it was a string not an integer.
>
> In D56113#1345238 , @ABataev wrote:
>
> > In D56113#1345232 , @jdenny wrote:
> >
> > > In D56113#1345047 , @ABataev 
> > > wrote:
> > >
> > > > In D56113#1344210 , @jdenny 
> > > > wrote:
> > > >
> > > > > In D56113#1342879 , @ABataev 
> > > > > wrote:
> > > > >
> > > > > > But you will need another serie of patches for `reduction` and 
> > > > > > `linear` clauses to update their error messages.
> > > > >
> > > > >
> > > > > Those already had their own checks for const.  I'm not aware of any 
> > > > > cases not handled, and the test suite does pass after my patch.
> > > >
> > > >
> > > > Yes, they have the checks for constness, but you need to  update those 
> > > > checks to emit this new error message for const values with mutable 
> > > > fields.
> > >
> > >
> > > I believe you mean we should reuse `rejectConstNotMutableType` for 
> > > `reduction` and `linear` rather than leave some of its implementation 
> > > duplicated for those.
> >
>
>
> For reductions, this will change the message from "const-qualified list item 
> cannot be reduction" to "const-qualified variable cannot be reduction".  Is 
> that OK?  (There are many tests to update, so I want to ask first.)


Mmmm. For the reductions better to keep the original message, because the list 
items also might be the array sections, not only the variables.


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

https://reviews.llvm.org/D56113



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

r350340 - Validate -add-plugin arguments.

[Nico Weber via cfe-commits]

Author: nico
Date: Thu Jan  3 10:26:06 2019
New Revision: 350340

URL: http://llvm.org/viewvc/llvm-project?rev=350340&view=rev
Log:
Validate -add-plugin arguments.

-plugin already prints an error if the name of an unknown plugin is passed.
-add-plugin used to silently ignore that, now it errors too.

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

Added:
cfe/trunk/test/Frontend/plugin-unknown.c
Modified:
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=350340&r1=350339&r2=350340&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Thu Jan  3 10:26:06 2019
@@ -33,6 +33,7 @@
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/Frontend/LangStandard.h"
 #include "clang/Frontend/MigratorOptions.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
@@ -1663,7 +1664,20 @@ static InputKind ParseFrontendArgs(Front
 Opts.ProgramAction = frontend::PluginAction;
 Opts.ActionName = A->getValue();
   }
-  Opts.AddPluginActions = Args.getAllArgValues(OPT_add_plugin);
+  for (const std::string &Arg : Args.getAllArgValues(OPT_add_plugin)) {
+bool Found = false;
+for (FrontendPluginRegistry::iterator it = FrontendPluginRegistry::begin(),
+  ie = FrontendPluginRegistry::end();
+ it != ie; ++it) {
+  if (it->getName() == Arg)
+Found = true;
+}
+if (!Found) {
+  Diags.Report(diag::err_fe_invalid_plugin_name) << Arg;
+  continue;
+}
+Opts.AddPluginActions.push_back(Arg);
+  }
   for (const auto *AA : Args.filtered(OPT_plugin_arg))
 Opts.PluginArgs[AA->getValue(0)].emplace_back(AA->getValue(1));
 

Added: cfe/trunk/test/Frontend/plugin-unknown.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/plugin-unknown.c?rev=350340&view=auto
==
--- cfe/trunk/test/Frontend/plugin-unknown.c (added)
+++ cfe/trunk/test/Frontend/plugin-unknown.c Thu Jan  3 10:26:06 2019
@@ -0,0 +1,5 @@
+// RUN: not %clang_cc1 -plugin asdf %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -add-plugin asdf %s 2>&1 | FileCheck --check-prefix=ADD 
%s
+
+// CHECK: unable to find plugin 'asdf'
+// ADD: unable to find plugin 'asdf'


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

[PATCH] D56273: Validate -add-plugin arguments.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC350340: Validate -add-plugin arguments. (authored by nico, 
committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56273?vs=180085&id=180099#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D56273

Files:
  lib/Frontend/CompilerInvocation.cpp
  test/Frontend/plugin-unknown.c


Index: test/Frontend/plugin-unknown.c
===
--- test/Frontend/plugin-unknown.c
+++ test/Frontend/plugin-unknown.c
@@ -0,0 +1,5 @@
+// RUN: not %clang_cc1 -plugin asdf %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -add-plugin asdf %s 2>&1 | FileCheck --check-prefix=ADD 
%s
+
+// CHECK: unable to find plugin 'asdf'
+// ADD: unable to find plugin 'asdf'
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -33,6 +33,7 @@
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/Frontend/LangStandard.h"
 #include "clang/Frontend/MigratorOptions.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
@@ -1663,7 +1664,20 @@
 Opts.ProgramAction = frontend::PluginAction;
 Opts.ActionName = A->getValue();
   }
-  Opts.AddPluginActions = Args.getAllArgValues(OPT_add_plugin);
+  for (const std::string &Arg : Args.getAllArgValues(OPT_add_plugin)) {
+bool Found = false;
+for (FrontendPluginRegistry::iterator it = FrontendPluginRegistry::begin(),
+  ie = FrontendPluginRegistry::end();
+ it != ie; ++it) {
+  if (it->getName() == Arg)
+Found = true;
+}
+if (!Found) {
+  Diags.Report(diag::err_fe_invalid_plugin_name) << Arg;
+  continue;
+}
+Opts.AddPluginActions.push_back(Arg);
+  }
   for (const auto *AA : Args.filtered(OPT_plugin_arg))
 Opts.PluginArgs[AA->getValue(0)].emplace_back(AA->getValue(1));
 


Index: test/Frontend/plugin-unknown.c
===
--- test/Frontend/plugin-unknown.c
+++ test/Frontend/plugin-unknown.c
@@ -0,0 +1,5 @@
+// RUN: not %clang_cc1 -plugin asdf %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -add-plugin asdf %s 2>&1 | FileCheck --check-prefix=ADD %s
+
+// CHECK: unable to find plugin 'asdf'
+// ADD: unable to find plugin 'asdf'
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -33,6 +33,7 @@
 #include "clang/Frontend/DependencyOutputOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/Frontend/LangStandard.h"
 #include "clang/Frontend/MigratorOptions.h"
 #include "clang/Frontend/PreprocessorOutputOptions.h"
@@ -1663,7 +1664,20 @@
 Opts.ProgramAction = frontend::PluginAction;
 Opts.ActionName = A->getValue();
   }
-  Opts.AddPluginActions = Args.getAllArgValues(OPT_add_plugin);
+  for (const std::string &Arg : Args.getAllArgValues(OPT_add_plugin)) {
+bool Found = false;
+for (FrontendPluginRegistry::iterator it = FrontendPluginRegistry::begin(),
+  ie = FrontendPluginRegistry::end();
+ it != ie; ++it) {
+  if (it->getName() == Arg)
+Found = true;
+}
+if (!Found) {
+  Diags.Report(diag::err_fe_invalid_plugin_name) << Arg;
+  continue;
+}
+Opts.AddPluginActions.push_back(Arg);
+  }
   for (const auto *AA : Args.filtered(OPT_plugin_arg))
 Opts.PluginArgs[AA->getValue(0)].emplace_back(AA->getValue(1));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

As you can see, lld doesn't really have any host-dependent knowledge so far, 
even though there are a few OSes that use lld as the default linker. We've 
added host-dependent knowledge to the compiler driver instead of to the linker 
for various operating systems. I guess that we could do the same thing for 
NetBSD, no?


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

It's an option but Joerg insists on using lld standalone.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D56113#1345337 , @ABataev wrote:

> > In D56113#1345238 , @ABataev wrote:
> > 
> >> In D56113#1345232 , @jdenny wrote:
> >>
> >> > In D56113#1345047 , @ABataev 
> >> > wrote:
> >> >
> >> > > In D56113#1344210 , @jdenny 
> >> > > wrote:
> >> > >
> >> > > > In D56113#1342879 , 
> >> > > > @ABataev wrote:
> >> > > >
> >> > > > > But you will need another serie of patches for `reduction` and 
> >> > > > > `linear` clauses to update their error messages.
> >> > > >
> >> > > >
> >> > > > Those already had their own checks for const.  I'm not aware of any 
> >> > > > cases not handled, and the test suite does pass after my patch.
> >> > >
> >> > >
> >> > > Yes, they have the checks for constness, but you need to  update those 
> >> > > checks to emit this new error message for const values with mutable 
> >> > > fields.
> >> >
> >> >
> >> > I believe you mean we should reuse `rejectConstNotMutableType` for 
> >> > `reduction` and `linear` rather than leave some of its implementation 
> >> > duplicated for those.
> >>
> > 
> > 
> > For reductions, this will change the message from "const-qualified list 
> > item cannot be reduction" to "const-qualified variable cannot be 
> > reduction".  Is that OK?  (There are many tests to update, so I want to ask 
> > first.)
>
> Mmmm. For the reductions better to keep the original message, because the 
> list items also might be the array sections, not only the variables.


I need to add a parameter to `rejectConstNotMutableType` to specify whether 
decl/def notes are included, and I can reuse that parameter for this purpose.  
Whether the diagnostic says "list item" or "variable" will then depend on what 
the list item is (variable, array section, etc.) rather than what the clause is 
(private, reduction, etc.).  I think that's ok.


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

https://reviews.llvm.org/D56113



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

Actually I find it frustrating and unfair as GNU ld doesn't have builtin 
knowledge either.. it's passed with gross 'super hack' comments from build 
scripts... but we are forced to push it to lld in order to move on.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D56113#1345372 , @jdenny wrote:

> In D56113#1345337 , @ABataev wrote:
>
> > > In D56113#1345238 , @ABataev 
> > > wrote:
> > > 
> > >> In D56113#1345232 , @jdenny 
> > >> wrote:
> > >>
> > >> > In D56113#1345047 , @ABataev 
> > >> > wrote:
> > >> >
> > >> > > In D56113#1344210 , @jdenny 
> > >> > > wrote:
> > >> > >
> > >> > > > In D56113#1342879 , 
> > >> > > > @ABataev wrote:
> > >> > > >
> > >> > > > > But you will need another serie of patches for `reduction` and 
> > >> > > > > `linear` clauses to update their error messages.
> > >> > > >
> > >> > > >
> > >> > > > Those already had their own checks for const.  I'm not aware of 
> > >> > > > any cases not handled, and the test suite does pass after my patch.
> > >> > >
> > >> > >
> > >> > > Yes, they have the checks for constness, but you need to  update 
> > >> > > those checks to emit this new error message for const values with 
> > >> > > mutable fields.
> > >> >
> > >> >
> > >> > I believe you mean we should reuse `rejectConstNotMutableType` for 
> > >> > `reduction` and `linear` rather than leave some of its implementation 
> > >> > duplicated for those.
> > >>
> > > 
> > > 
> > > For reductions, this will change the message from "const-qualified list 
> > > item cannot be reduction" to "const-qualified variable cannot be 
> > > reduction".  Is that OK?  (There are many tests to update, so I want to 
> > > ask first.)
> >
> > Mmmm. For the reductions better to keep the original message, because the 
> > list items also might be the array sections, not only the variables.
>
>
> I need to add a parameter to `rejectConstNotMutableType` to specify whether 
> decl/def notes are included, and I can reuse that parameter for this purpose. 
>  Whether the diagnostic says "list item" or "variable" will then depend on 
> what the list item is (variable, array section, etc.) rather than what the 
> clause is (private, reduction, etc.).  I think that's ok.


Ok, sounds good


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

https://reviews.llvm.org/D56113



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

[PATCH] D56271: [SemaCXX] Fix ICE for unexpanded parameter pack

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

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

Looks good, thank you!


Repository:
  rC Clang

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

https://reviews.llvm.org/D56271



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

I sympathize and understand your frustration, but I don't know what I can do to 
you. The thing that lld doesn't have host-specific config is a policy matter 
we've been maintaining so far for all operating systems. I don't think NetBSD 
should be the only exception of the policy. In addition to that, I strongly 
believe the fact that lld's behavior only depends on the command line options 
given to the linker is easier to understand and generally a good thing.

Joerg, what is special about NetBSD?


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D56215#1345400 , @ruiu wrote:

> Joerg, what is special about NetBSD?


Nothing.

But the result is that we don't have GNU gold either and are stuck with 40min 
linking times of LLVM. It's destructive to productivity and damages any LLVM 
related development.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

let me look further into that, it works for std::function but not for const 
std::function or std::function &,  const std::function &


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

https://reviews.llvm.org/D55433



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Talking from the perspective of having had to deal with thousands of packages 
in pkgsrc over the years: it is naive to believe that there isn't a lot of 
software that calls the linker directly for various reasons. As such, it is 
very important to have a useful configuration in the linker by default. Such a 
configuration is by its very nature target specific. This doesn't mean it can't 
be done in a cross-compiler friendly way, on the contrary. Over the years 
NetBSD has been pushing its toolchain to be as similar for the native build and 
a cross-target as reasonable possible. Modulo the build time choices in the 
config.h sense, the only difference between the native and cross tools is the 
built-in default of the former.

Clang works extremely well in that regard and perfectly supports a universal 
toolchain. LLD should as well. Consistent behavior of ELF across different OS 
is a fiction, as such some OS and/or CPU family specific defaults are naturally 
different. This can be grouped in general:
(1) The target operating system specified either from the builtin default, the 
name of the tool (triple-ld), an option like clang's --target or if desirable 
the name of the emulation when specified. I don't have a strong reason for 
supporting the last as it is typically not unique even with binutils and more a 
case of historic legacy than useful feature. For the perspective of supporting 
both native and cross toolchains, the first three options are enough and when 
using the compiler driver, the first two will generally work fine.
(2) The target CPU family. While it can be partially guessed from the object 
files, there are fun expects. ARM instruction encodings are one of the 
pecularities a linker has to deal with for example. Is BX a valid jump 
instruction?
(3) Whether the target CPU family is the primary ABI. This can generally not be 
determined from the object files and can affect the default search paths. Hard 
vs soft floating point is a good example here. Other cases are easier like 
linking i386 binaries on x86_64. N32 vs N64 on MIPS is more difficult again. 
This is a bit tricky in that it often can be drived only from the emulation 
name.

In terms of architecture, it doesn't need much, but it needs some thought. 
Identifying the target OS is likely the easiest. Most of the rest is OS 
specific adjustment. Having access to the binary name and the the arguments 
should be enough though. Doing it properly avoids the fun of having to patch a 
lot of software without costing that much in terms of code.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

[PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

I should add that this is not just about reproducible builds but about code 
size as mentioned by @NSProgrammer. There are ways to cheat with builtins but 
the problem is, entire __FILE__ string is still emitted into read-only data, 
despite the constexpressiveness of the value. The common way of getting the 
short filename in code would the the following (the `#else` case) which is a 
constant evaluated but has a side effect of dumping the full paths in rodata 
regardless (ICF seems unable to clean them up even in full LTO builds).

  #ifdef __clang_extension_generate
  #define OS_CUR_FILENAME  __generate("file!0")
  #else
  #define OS_CUR_FILENAME (__builtin_strrchr(__FILE__, '/') ? \
  __builtin_strrchr(__FILE__, '/') + 1 : __FILE__)
  #endif

The extension on the other hand avoids that. So it's a win for code size and 
reproducible builds. I would urge reconsidering something like `__FILE_NAME__`, 
or whatever name is preferable for just getting the last path component at 
preprocessor stage, despite it being poorly received by other developers the 
last time it seems to be a feature wanted by consumers. I would urge 
maintainers to reply to get another consensus regarding this since my stance is 
still the same, if a "missing" feature is widely hacked around, it's clearly 
desirable. While I understand that a lot have an aversion to nonstandard 
Clang-specific extensions, as long as a feature check is available it should be 
down to the consumer to decide if they want to make use of such extensions.


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

https://reviews.llvm.org/D17741



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

[PATCH] D56226: [clang-format] square parens that are followed by '=' are not Objective-C message sends

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 180110.
arphaman added a comment.

Do not annotate one token `[  ]` expressions as Objective-C message sends 
as suggested by Duncan.


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

https://reviews.llvm.org/D56226

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -166,6 +166,20 @@
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 }
 
+TEST(FormatTestObjCStyle, AvoidDetectingDesignatedInitializersAsObjCInHeaders) 
{
+  auto Style = getStyle("LLVM", "a.h", "none",
+"static const char *names[] = {[0] = \"foo\",\n"
+"[kBar] = \"bar\"};");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+
+  Style = getStyle("LLVM", "a.h", "none",
+   "static const char *names[] = {[0] EQ \"foo\",\n"
+   "[kBar] EQ \"bar\"};");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+}
+
 TEST_F(FormatTestObjC, FormatObjCTryCatch) {
   verifyFormat("@try {\n"
"  f();\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -495,9 +495,13 @@
   if (CurrentToken->is(tok::r_square)) {
 if (IsCpp11AttributeSpecifier)
   CurrentToken->Type = TT_AttributeSquare;
-else if (CurrentToken->Next && CurrentToken->Next->is(tok::l_paren) &&
+else if (((CurrentToken->Next &&
+   (CurrentToken->Next->is(tok::l_paren) ||
+CurrentToken->Next->is(tok::equal))) ||
+  (CurrentToken->Previous->Previous == Left)) &&
  Left->is(TT_ObjCMethodExpr)) {
-  // An ObjC method call is rarely followed by an open parenthesis.
+  // An ObjC method call is rarely followed by an open parenthesis or
+  // an assignment. It also usually contains more than one token.
   // FIXME: Do we incorrectly label ":" with this?
   StartsObjCMethodExpr = false;
   Left->Type = TT_Unknown;


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -166,6 +166,20 @@
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 }
 
+TEST(FormatTestObjCStyle, AvoidDetectingDesignatedInitializersAsObjCInHeaders) {
+  auto Style = getStyle("LLVM", "a.h", "none",
+"static const char *names[] = {[0] = \"foo\",\n"
+"[kBar] = \"bar\"};");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+
+  Style = getStyle("LLVM", "a.h", "none",
+   "static const char *names[] = {[0] EQ \"foo\",\n"
+   "[kBar] EQ \"bar\"};");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+}
+
 TEST_F(FormatTestObjC, FormatObjCTryCatch) {
   verifyFormat("@try {\n"
"  f();\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -495,9 +495,13 @@
   if (CurrentToken->is(tok::r_square)) {
 if (IsCpp11AttributeSpecifier)
   CurrentToken->Type = TT_AttributeSquare;
-else if (CurrentToken->Next && CurrentToken->Next->is(tok::l_paren) &&
+else if (((CurrentToken->Next &&
+   (CurrentToken->Next->is(tok::l_paren) ||
+CurrentToken->Next->is(tok::equal))) ||
+  (CurrentToken->Previous->Previous == Left)) &&
  Left->is(TT_ObjCMethodExpr)) {
-  // An ObjC method call is rarely followed by an open parenthesis.
+  // An ObjC method call is rarely followed by an open parenthesis or
+  // an assignment. It also usually contains more than one token.
   // FIXME: Do we incorrectly label ":" with this?
   StartsObjCMethodExpr = false;
   Left->Type = TT_Unknown;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

We will prepare a patch for i386 x86_64 only for now. Until we will get lld 
fully functional on NetBSD/amd64 we will skip other architectures.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Michał Górny via Phabricator via cfe-commits]

mgorny updated this revision to Diff 180114.
mgorny edited the summary of this revision.
mgorny added a comment.
Herald added subscribers: atanasyan, sdardis.

Updated to use clang's libdir logic.


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

https://reviews.llvm.org/D56215

Files:
  ELF/Driver.cpp
  ELF/Driver.h


Index: ELF/Driver.h
===
--- ELF/Driver.h
+++ ELF/Driver.h
@@ -32,6 +32,7 @@
 
 private:
   void readConfigs(llvm::opt::InputArgList &Args);
+  void appendDefaultSearchPaths();
   void createFiles(llvm::opt::InputArgList &Args);
   void inferMachineType();
   template  void link(llvm::opt::InputArgList &Args);
Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -365,6 +365,28 @@
   error("unknown -z value: " + StringRef(Arg->getValue()));
 }
 
+void LinkerDriver::appendDefaultSearchPaths() {
+#if defined(__NetBSD__)
+  // NetBSD driver relies on the linker knowing the default search paths.
+  // Please keep this in sync with clang/lib/Driver/ToolChains/NetBSD.cpp
+  // (NetBSD::NetBSD constructor)
+  switch (Config->EMachine) {
+case EM_386:
+  Config->SearchPaths.push_back("/usr/lib/i386");
+  break;
+case EM_MIPS:
+  if (Config->EKind == ELF64LEKind || Config->EKind == ELF64BEKind)
+Config->SearchPaths.push_back("/usr/lib/64");
+  // TODO: how to detect o32?
+  break;
+case EM_PPC64:
+  break;
+// TODO: add *nbsd* emulations when supported
+  }
+  Config->SearchPaths.push_back("/usr/lib");
+#endif
+}
+
 void LinkerDriver::main(ArrayRef ArgsArr) {
   ELFOptTable Parser;
   opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
@@ -412,6 +434,7 @@
 
   readConfigs(Args);
   checkZOptions(Args);
+  appendDefaultSearchPaths();
 
   // The behavior of -v or --version is a bit strange, but this is
   // needed for compatibility with GNU linkers.


Index: ELF/Driver.h
===
--- ELF/Driver.h
+++ ELF/Driver.h
@@ -32,6 +32,7 @@
 
 private:
   void readConfigs(llvm::opt::InputArgList &Args);
+  void appendDefaultSearchPaths();
   void createFiles(llvm::opt::InputArgList &Args);
   void inferMachineType();
   template  void link(llvm::opt::InputArgList &Args);
Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -365,6 +365,28 @@
   error("unknown -z value: " + StringRef(Arg->getValue()));
 }
 
+void LinkerDriver::appendDefaultSearchPaths() {
+#if defined(__NetBSD__)
+  // NetBSD driver relies on the linker knowing the default search paths.
+  // Please keep this in sync with clang/lib/Driver/ToolChains/NetBSD.cpp
+  // (NetBSD::NetBSD constructor)
+  switch (Config->EMachine) {
+case EM_386:
+  Config->SearchPaths.push_back("/usr/lib/i386");
+  break;
+case EM_MIPS:
+  if (Config->EKind == ELF64LEKind || Config->EKind == ELF64BEKind)
+Config->SearchPaths.push_back("/usr/lib/64");
+  // TODO: how to detect o32?
+  break;
+case EM_PPC64:
+  break;
+// TODO: add *nbsd* emulations when supported
+  }
+  Config->SearchPaths.push_back("/usr/lib");
+#endif
+}
+
 void LinkerDriver::main(ArrayRef ArgsArr) {
   ELFOptTable Parser;
   opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
@@ -412,6 +434,7 @@
 
   readConfigs(Args);
   checkZOptions(Args);
+  appendDefaultSearchPaths();
 
   // The behavior of -v or --version is a bit strange, but this is
   // needed for compatibility with GNU linkers.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin marked an inline comment as done.
alexey.lapshin added inline comments.



Comment at: test/Sema/pragma-pipeline.cpp:3
+
+#pragma clang loop pipeline(disable) /* expected-error {{expected 
unqualified-id}} */
+int main() {

aaron.ballman wrote:
> I think this error is pretty unfortunate -- it doesn't really help the user 
> to understand what's wrong with their code. Can it be improved?
Agreed, this does not look very informative. It surely can be improved. Though 
it looks like not related to my fix. My fix is to add two additional loop 
hints. That kind of diagnostic related to all loop hints "clang loop". I.e. all 
pragmas "clang loop" can be checked for various cases and for better 
diagnostic. It looks like separate task. But OK, I will check that case.


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

https://reviews.llvm.org/D55710



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added inline comments.



Comment at: ELF/Driver.cpp:377
+  break;
+case EM_MIPS:
+  if (Config->EKind == ELF64LEKind || Config->EKind == ELF64BEKind)

Please drop MIPS/PPC for now.


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

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added inline comments.



Comment at: ELF/Driver.cpp:369
+void LinkerDriver::appendDefaultSearchPaths() {
+#if defined(__NetBSD__)
+  // NetBSD driver relies on the linker knowing the default search paths.

is it possible to use some Triple NetBSD target here?


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

https://reviews.llvm.org/D56215



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Michał Górny via Phabricator via cfe-commits]

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



Comment at: ELF/Driver.cpp:369
+void LinkerDriver::appendDefaultSearchPaths() {
+#if defined(__NetBSD__)
+  // NetBSD driver relies on the linker knowing the default search paths.

krytarowski wrote:
> is it possible to use some Triple NetBSD target here?
Technically, you can get triple from process name but that obviously works only 
when lld is run as `$CHOST-lld` and not plain `lld`, which is not really the 
case right now.


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

https://reviews.llvm.org/D56215



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D56113#1345238 , @ABataev wrote:

> >>> By the way, is there any value to keeping the predetermined shared for 
> >>> const if -openmp-version=3.1 or earlier?
> >> 
> >> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 it 
> >> will have the value `31`.
> > 
> > How far back should we take this?  I'm inclined to check for `30` and `31` 
> > only and assume anything else is newer, but let me know if we need to check 
> > for earlier versions.
>
> I think `<= 31` is good. Clang always supported only OpenMP 3.1 and higher.


I'm planning to let this affect the behavior of `default(none)` (predetermined 
shared means no explicit attribute is needed).

I don't plan to let it affect which version of the diagnostics are produced.  I 
think the newer diagnostics are clearer even though they are not expressed 
precisely in terms of 3.1 semantics.  Moreover, there are less cases to test 
this way.  Let me know if you think this is wrong.  If you want to review the 
updated patch first, I'll be posting it soon.

By the way, LangOpts.OpenMP currently defaults to 0.  Should it?


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

https://reviews.llvm.org/D56113



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

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Michał Górny via Phabricator via cfe-commits]

mgorny updated this revision to Diff 180121.
mgorny edited the summary of this revision.
mgorny added a comment.

Removed non-x86.


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

https://reviews.llvm.org/D56215

Files:
  ELF/Driver.cpp
  ELF/Driver.h


Index: ELF/Driver.h
===
--- ELF/Driver.h
+++ ELF/Driver.h
@@ -32,6 +32,7 @@
 
 private:
   void readConfigs(llvm::opt::InputArgList &Args);
+  void appendDefaultSearchPaths();
   void createFiles(llvm::opt::InputArgList &Args);
   void inferMachineType();
   template  void link(llvm::opt::InputArgList &Args);
Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -365,6 +365,23 @@
   error("unknown -z value: " + StringRef(Arg->getValue()));
 }
 
+void LinkerDriver::appendDefaultSearchPaths() {
+#if defined(__NetBSD__)
+  // NetBSD driver relies on the linker knowing the default search paths.
+  // Please keep this in sync with clang/lib/Driver/ToolChains/NetBSD.cpp
+  // (NetBSD::NetBSD constructor)
+  switch (Config->EMachine) {
+case EM_386:
+  Config->SearchPaths.push_back("/usr/lib/i386");
+  break;
+case EM_X86_64:
+  break;
+// TODO: support non-x86 architectures
+  }
+  Config->SearchPaths.push_back("/usr/lib");
+#endif
+}
+
 void LinkerDriver::main(ArrayRef ArgsArr) {
   ELFOptTable Parser;
   opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
@@ -412,6 +429,7 @@
 
   readConfigs(Args);
   checkZOptions(Args);
+  appendDefaultSearchPaths();
 
   // The behavior of -v or --version is a bit strange, but this is
   // needed for compatibility with GNU linkers.


Index: ELF/Driver.h
===
--- ELF/Driver.h
+++ ELF/Driver.h
@@ -32,6 +32,7 @@
 
 private:
   void readConfigs(llvm::opt::InputArgList &Args);
+  void appendDefaultSearchPaths();
   void createFiles(llvm::opt::InputArgList &Args);
   void inferMachineType();
   template  void link(llvm::opt::InputArgList &Args);
Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -365,6 +365,23 @@
   error("unknown -z value: " + StringRef(Arg->getValue()));
 }
 
+void LinkerDriver::appendDefaultSearchPaths() {
+#if defined(__NetBSD__)
+  // NetBSD driver relies on the linker knowing the default search paths.
+  // Please keep this in sync with clang/lib/Driver/ToolChains/NetBSD.cpp
+  // (NetBSD::NetBSD constructor)
+  switch (Config->EMachine) {
+case EM_386:
+  Config->SearchPaths.push_back("/usr/lib/i386");
+  break;
+case EM_X86_64:
+  break;
+// TODO: support non-x86 architectures
+  }
+  Config->SearchPaths.push_back("/usr/lib");
+#endif
+}
+
 void LinkerDriver::main(ArrayRef ArgsArr) {
   ELFOptTable Parser;
   opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
@@ -412,6 +429,7 @@
 
   readConfigs(Args);
   checkZOptions(Args);
+  appendDefaultSearchPaths();
 
   // The behavior of -v or --version is a bit strange, but this is
   // needed for compatibility with GNU linkers.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D56113#1345529 , @jdenny wrote:

> In D56113#1345238 , @ABataev wrote:
>
> > >>> By the way, is there any value to keeping the predetermined shared for 
> > >>> const if -openmp-version=3.1 or earlier?
> > >> 
> > >> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 it 
> > >> will have the value `31`.
> > > 
> > > How far back should we take this?  I'm inclined to check for `30` and 
> > > `31` only and assume anything else is newer, but let me know if we need 
> > > to check for earlier versions.
> >
> > I think `<= 31` is good. Clang always supported only OpenMP 3.1 and higher.
>
>
> I'm planning to let this affect the behavior of `default(none)` 
> (predetermined shared means no explicit attribute is needed).
>
> I don't plan to let it affect which version of the diagnostics are produced.  
> I think the newer diagnostics are clearer even though they are not expressed 
> precisely in terms of 3.1 semantics.  Moreover, there are less cases to test 
> this way.  Let me know if you think this is wrong.  If you want to review the 
> updated patch first, I'll be posting it soon.


It would be good if could keep the original implementation for 3.1.

> By the way, LangOpts.OpenMP currently defaults to 0.  Should it?

Yes, it means it is disabled by default.


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

https://reviews.llvm.org/D56113



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: test/Sema/pragma-pipeline.cpp:3
+
+#pragma clang loop pipeline(disable) /* expected-error {{expected 
unqualified-id}} */
+int main() {

alexey.lapshin wrote:
> aaron.ballman wrote:
> > I think this error is pretty unfortunate -- it doesn't really help the user 
> > to understand what's wrong with their code. Can it be improved?
> Agreed, this does not look very informative. It surely can be improved. 
> Though it looks like not related to my fix. My fix is to add two additional 
> loop hints. That kind of diagnostic related to all loop hints "clang loop". 
> I.e. all pragmas "clang loop" can be checked for various cases and for better 
> diagnostic. It looks like separate task. But OK, I will check that case.
That's a fair point -- I'm fine with doing that work in a separate patch. I 
don't think we need to hold this patch up further for it, anyway.


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

https://reviews.llvm.org/D55710



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

[PATCH] D56288: [ELF] Do not enable 'new dtags' on NetBSD by default

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.
mgorny added reviewers: ruiu, krytarowski, joerg.
Herald added subscribers: llvm-commits, arichardson, emaste.
Herald added a reviewer: espindola.

NetBSD strongly prefers using 'old' dtag (RPATH) over Linux-specific
RUNPATH, and did not support the latter until very recently.  Therefore,
defaulting to RUNPATH causes lld to produce non-working executables
by default.  Switch the default appropriately when building on NetBSD.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D56288

Files:
  ELF/Driver.cpp


Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -766,6 +766,14 @@
 
 // Initializes Config members by the command line options.
 void LinkerDriver::readConfigs(opt::InputArgList &Args) {
+#if defined(__NetBSD__)
+// NetBSD prefers RPATH, and does not allow RUNPATH until very recently
+#  define DEFAULT_NEW_DTAGS false
+#else
+#  define DEFAULT_NEW_DTAGS true
+#endif
+
+
   errorHandler().Verbose = Args.hasArg(OPT_verbose);
   errorHandler().FatalWarnings =
   Args.hasFlag(OPT_fatal_warnings, OPT_no_fatal_warnings, false);
@@ -797,7 +805,8 @@
   Config->CallGraphProfileSort = Args.hasFlag(
   OPT_call_graph_profile_sort, OPT_no_call_graph_profile_sort, true);
   Config->EnableNewDtags =
-  Args.hasFlag(OPT_enable_new_dtags, OPT_disable_new_dtags, true);
+  Args.hasFlag(OPT_enable_new_dtags, OPT_disable_new_dtags,
+   DEFAULT_NEW_DTAGS);
   Config->Entry = Args.getLastArgValue(OPT_entry);
   Config->ExecuteOnly =
   Args.hasFlag(OPT_execute_only, OPT_no_execute_only, false);


Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -766,6 +766,14 @@
 
 // Initializes Config members by the command line options.
 void LinkerDriver::readConfigs(opt::InputArgList &Args) {
+#if defined(__NetBSD__)
+// NetBSD prefers RPATH, and does not allow RUNPATH until very recently
+#  define DEFAULT_NEW_DTAGS false
+#else
+#  define DEFAULT_NEW_DTAGS true
+#endif
+
+
   errorHandler().Verbose = Args.hasArg(OPT_verbose);
   errorHandler().FatalWarnings =
   Args.hasFlag(OPT_fatal_warnings, OPT_no_fatal_warnings, false);
@@ -797,7 +805,8 @@
   Config->CallGraphProfileSort = Args.hasFlag(
   OPT_call_graph_profile_sort, OPT_no_call_graph_profile_sort, true);
   Config->EnableNewDtags =
-  Args.hasFlag(OPT_enable_new_dtags, OPT_disable_new_dtags, true);
+  Args.hasFlag(OPT_enable_new_dtags, OPT_disable_new_dtags,
+   DEFAULT_NEW_DTAGS);
   Config->Entry = Args.getLastArgValue(OPT_entry);
   Config->ExecuteOnly =
   Args.hasFlag(OPT_execute_only, OPT_no_execute_only, false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via cfe-commits]

On Thu, Jan 03, 2019 at 06:34:22PM +, Kamil Rytarowski via Phabricator via 
cfe-commits wrote:
> krytarowski added a comment.
> 
> Actually I find it frustrating and unfair as GNU ld doesn't have builtin
> knowledge either.. it's passed with gross 'super hack' comments from build 
> scripts... but we are forced to push it to lld in order to move on.

I'm puzzled. Seriously, when was the last time you actually checked how
much customization contains on a per-OS base in GNU ld? Yes, I'm
including the various build scripts because GNU ld is generally build by
a mix of hard-coded logic in the tool itself and various adjustments in
the linker scripts it is shipped with. But they are all a builtin part
of GNU ld as far as the end user is concerned. It is pretty much
irrelevant from a point of functionality where that logic is, but
skipping is a serious usability issue.

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

Re: [PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

[Joerg Sonnenberger via cfe-commits]

On Thu, Jan 03, 2019 at 06:58:41PM +, Kamil Rytarowski via Phabricator 
wrote:
> But the result is that we don't have GNU gold either and are stuck with
> 40min linking times of LLVM. It's destructive to productivity and
> damages any LLVM related development.

The reason noone cared much about GNU gold is that it supports only a
limited set of platforms and forces a lot of modern GNU "innovations"
without any chance of fixing them. To a degree, both concerns apply to
lld as well, but reasonable well integrated LTO support with Clang
provides at least something in return. I have no idea about your link
times, the only situation where linking is taking a really significant
chunk of time is for full debug builds and the general solution for that
is DWARF fission.

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