[PATCH] D68115: Zero initialize padding in unions

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D68115#1686887 , @vitalybuka wrote:

> I would be happy to update the patch to enable it only for 
> -ftrivial-auto-var-init=pattern, if we want "bumper" version.


It seems to be a separable feature (although it does interact with 
`-ftrivial-auto-init=pattern`). That option also provides guardrails for 
non-unions, and "bumper guardrails" for unions can be a useful feature without 
the non-union guardrails.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68115



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

[PATCH] D68115: Zero initialize padding in unions

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

> The patch as-is moves past the scope of the `-ftrivial-auto-var-init` feature.

That's because I had misunderstanding of the standard. I would be happy to 
update the patch to enable it only for -ftrivial-auto-var-init=pattern, if we 
want "bumper" version. For "non-bumper" we don't need a patch at all.
I don't have strong opinion which way to go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68115



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

[PATCH] D68115: Zero initialize padding in unions

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

In D68115#1686837 , @jfb wrote:

> The entire point of this feature is to add guardrails to the language. What 
> do people expect in the real world? Is there a cost to meeting these 
> expectations? If we put the pattern (0x00 or 0xaa) in the technically undef 
> space, what comes out?


As a user I would prefer zeros there :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68115



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

[PATCH] D68115: Zero initialize padding in unions

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D68115#1686837 , @jfb wrote:

> The entire point of this feature is to add guardrails to the language.


I agree, and guardrails have a tendency to scratch paint if one drives against 
them.

> What do people expect in the real world? Is there a cost to meeting these 
> expectations?

The patch as-is moves past the scope of the `-ftrivial-auto-var-init` feature. 
The specific case I wrote the inline comment on is an instance where the 
initialization strategy appears deliberate and costs less space in the compiled 
binary than the case where the initialization strategy is hampered by trying to 
initialize bytes that are defined as holding indeterminate values. Paying for 
this extra space should require opting into (such as by using 
`-ftrivial-auto-var-init`).

> If we put the pattern (0x00 or 0xaa) in the technically undef space, what 
> comes out?

To extend the analogy, `0x00` seems to be the bumper car version in the context 
of the current discussion. Applications that have issues around uninitialized 
bytes in unions might be workable when using `0x00` as the pattern. With a 
non-bumper car pattern, it would become more clear to users when they are 
driving against the guardrails, so they aren't instead surprised when they fall 
off a cliff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68115



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

[PATCH] D68115: Zero initialize padding in unions

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

The entire point of this feature is to add guardrails to the language. What do 
people expect in the real world? Is there a cost to meeting these expectations? 
If we put the pattern (0x00 or 0xaa) in the technically undef space, what comes 
out?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68115



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

[PATCH] D68163: [analyzer][MallocChecker][NFC] Change the use of IdentifierInfo* to CallDescription

[Artem Dergachev via Phabricator via cfe-commits]

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

Fantastic, thanks!!




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:259
   /// calls.
   bool isCalled(const CallDescription ) const;
 

I don't fully understand how does overload resolution work in this case, maybe 
rename the original function?



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1006
   const FunctionDecl *FD = C.getCalleeDecl(CE);
-  if (!FD)
+  if (!FD || FD->getKind() != Decl::Function)
 return;

The `FD->getKind() != Decl::Function` part is super mega redundant here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68163



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

[PATCH] D68162: [analyzer][MallocChecker][NFC] Communicate the allocation family information to auxiliary functions with template parameters

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Thank you, fantastic finding!

> in fact we know it //compile time//

Yeah, but is it accidental or is there a good reason behind always having this 
information at compile time? 'Cause i don't want to restrict the code to always 
provide this information at compile time if we're not sure it'll always be able 
to provide it in compile time.

My mental model for this sort of stuff is something like this:

  CallDescriptionMap M = {
"malloc", {::MallocMemAux, AF_Malloc},
"alloca", {::MallocMemAux, AF_Alloca},
  };
  
  void checkPostCall(const CallEvent , CheckerContext ) {
if (Info *I = M.lookup(Call))
  I->first(I->second, C, Call);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68162



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

[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:378-379
+
+  using CheckFn = void (MallocChecker::*)(CheckerContext , const CallExpr 
*CE,
+  ProgramStateRef State) const;
+

Whenever i see a (`CE`, `State`) pair, it screams `CallEvent` to me. That said, 
i'm worried that `State` in these callbacks isn't necessarily equal to 
`C.getState()` (the latter, by the way, is always equal to the `CallEvent`'s 
`.getState()` - that's a relief, right?), so if you'll ever be in the mood to 
check that, that'd be great :)



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:397-398
+  CallDescriptionMap NonFreeingMemFnMap{
+  {{"alloca", 1}, ::checkAlloca},
+  {{"_alloca", 1}, ::checkAlloca},
+  {{"malloc", 1}, ::checkMalloc},

I think `alloca` deserves `CDF_MaybeBuiltin`. This would also probably allow 
you to remove the second line.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:402-404
+  {{"strndup", 2}, ::checkStrdup},
+  {{"strdup", 1}, ::checkStrdup},
+  {{"_strdup", 1}, ::checkStrdup},

These are also builtins.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:407-408
+  {{"if_nameindex", 1}, ::checkIfNameIndex},
+  {{"wcsdup", 1}, ::checkStrdup},
+  {{"_wcsdup", 1}, ::checkStrdup},
+  {{"g_malloc", 1}, ::checkMalloc},

And these too.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1487
 
-  if (Att->getModule()->getName() !=
-  MemFunctionInfo.CD_malloc.getFunctionName())
+  if (Att->getModule()->getName() != "malloc")
 return nullptr;

Szelethus wrote:
> Ugh, no idea whats happening here, I'll be honest. I sweat to dig a bit 
> deeper to find out, though I didn't want to bother delaying the publishing 
> this patch for a feature not documented, or used anywhere.
I think this is about the (lack of) support for `__attribute__((malloc))`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68165



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

[PATCH] D68172: Don't install example analyzer plugins

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

In D68172#1686772 , @NoQ wrote:

> +@Szelethus because i'm a bit out-of-the-loop on plugins: i very vaguely 
> remember that we decided to put them into tests(?)


The plugins were in `test`, but D62445  
changed that. Another solution would have been to move them into the 
`unittests` directory. But other example plugins are also part of the ordinary 
source tree, see for example `llvm/lib/Transforms/Hello`. In fact this change 
is based on LLVMHello, which also has `BUILDTREE_ONLY`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68172



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

[PATCH] D68172: Don't install example analyzer plugins

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a reviewer: Szelethus.
NoQ added a comment.

+@Szelethus because i'm a bit out-of-the-loop on plugins: i very vaguely 
remember that we decided to put them into tests(?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68172



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

[PATCH] D68115: Zero initialize padding in unions

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/init.c:197
   // CHECK-LABEL: @nonzeroPaddedUnionMemset(
-  // CHECK-NOT: store
-  // CHECK-NOT: memcpy
-  // CHECK: call void @llvm.memset.p0i8.i32(i8* {{.*}}, i8 -16, i32 36, i1 
false)
+  // CHECK: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 {{.*}}, i8* align 
4 {{.*}} [[INIT_PADDEDUNION]], {{.*}}, i32 36, i1 false)
 }

vitalybuka wrote:
> hubert.reinterpretcast wrote:
> > vitalybuka wrote:
> > > vitalybuka wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > This is C++ aggregate initialization and not value-initialization. 
> > > > > The wording you quoted from the C++ standard is for 
> > > > > zero-initialization, which might be part of value initialization, but 
> > > > > you have not shown that aggregate initialization of a union involves 
> > > > > zero-initialization of that union.
> > > > reading this more I don't see any evidence that either C++ or C 
> > > > requires padding initialization.
> > > > Reading this I expect that all function here should be equivalent 
> > > > https://godbolt.org/z/1O_9-e
> > > > But they are not. Clang and GCC initialized padding after the first 
> > > > member.
> > > So if go trough "aggregates" then nothing is said about padding in union.
> > > If we go trough "list-initialization" then value initialization should be 
> > > applied, part of which is zero initialization. 
> > > If so union padding should be initialized.
> > > 
> > > 
> > In C++14, the list in [dcl.init.list] starts with:
> > If T is an aggregate, aggregate initialization is performed.
> > 
> > The bullet in C++17 follows only a case for copying and then a case for 
> > string literals.
> > 
> > The bullet in the C++2a CD likewise, with an additional earlier bullet that 
> > also becomes aggregate initialization.
> > 
> > It is in C++11 where an empty list gets value-initialization treatment, and 
> > the next bullet goes to aggregate initialization.
> > 
> > The inline comment was not added to an empty list case.
> So if I understand you (and what I see in C++17 and C++11) then only C++11 
> will essentially requires zeros via value-initialization.
> And the rest (including C), requires only the first field to be initialized 
> and the tail is undefined.
> 
> 
That is my understanding. C++11 will require zeroes for `U u = { };` and `U 
u{}`. The tail is undefined in other cases with automatic storage duration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68115



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

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D67706#1685963 , @Szelethus wrote:

> It seems like this patch is diffed against your latest commit, not the master 
> branch.


Yeah, seems so. The code looks great tho, thanks!




Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:64
 public:
-  StreamChecker()
-  : CD_fopen("fopen"), CD_tmpfile("tmpfile"), CD_fclose("fclose", 1),
-CD_fread("fread", 4), CD_fwrite("fwrite", 4), CD_fseek("fseek", 3),
-CD_ftell("ftell", 1), CD_rewind("rewind", 1), CD_fgetpos("fgetpos", 2),
-CD_fsetpos("fsetpos", 2), CD_clearerr("clearerr", 1),
-CD_feof("feof", 1), CD_ferror("ferror", 1), CD_fileno("fileno", 1) {}
+  StreamChecker() = default;
 

Feel free to omit the constructor entirely. We almost never have constructors 
in our checkers.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:74-87
+  {{CDF_MaybeBuiltin, "fopen"}, ::evalFopen},
+  {{CDF_MaybeBuiltin, "tmpfile"}, ::evalTmpfile},
+  {{CDF_MaybeBuiltin, "fclose", 1}, ::evalFclose},
+  {{CDF_MaybeBuiltin, "fread", 4}, ::evalFread},
+  {{CDF_MaybeBuiltin, "fwrite", 4}, ::evalFwrite},
+  {{CDF_MaybeBuiltin, "fseek", 3}, ::evalFseek},
+  {{CDF_MaybeBuiltin, "ftell", 1}, ::evalFtell},

Are you sure these functions are actually sometimes implemented as builtins? I 
think they're required to be regular functions.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:90-106
+  FnCheck identifyCall(const CallEvent , CheckerContext ,
+   const CallExpr *CE) const;
+
+  void evalFopen(CheckerContext , const CallExpr *CE) const;
+  void evalTmpfile(CheckerContext , const CallExpr *CE) const;
+  void evalFclose(CheckerContext , const CallExpr *CE) const;
+  void evalFread(CheckerContext , const CallExpr *CE) const;

For most purposes it's more convenient to pass `CallEvent` around. The only 
moment when you actually need the `CallExpr` is when you're doing the binding 
for the return value, which happens once, so it's fine to call 
`.getOriginExpr()` directly at this point.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:125-127
   const auto *FD = dyn_cast_or_null(Call.getDecl());
   if (!FD || FD->getKind() != Decl::Function)
 return false;

(not your fault but before i forget) This check is actually redundant.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:273
+
+  if (SymbolRef Sym = RetVal.getAsSymbol()) {
+// if RetVal is not NULL, set the symbol's state to Opened.

(not your fault but before i forget) This condition is actually always true 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67706



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

[PATCH] D68172: Don't install example analyzer plugins

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added reviewers: hintonda, NoQ.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
a.sidorin, baloghadamsoftware, mgorny.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68172

Files:
  clang/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
  clang/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
  clang/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt


Index: clang/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
===
--- clang/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
+++ clang/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
@@ -3,7 +3,7 @@
   )
 
 set(LLVM_EXPORTED_SYMBOL_FILE 
${CMAKE_CURRENT_SOURCE_DIR}/SampleAnalyzerPlugin.exports)
-add_llvm_library(SampleAnalyzerPlugin MODULE MainCallChecker.cpp PLUGIN_TOOL 
clang)
+add_llvm_library(SampleAnalyzerPlugin MODULE BUILDTREE_ONLY 
MainCallChecker.cpp PLUGIN_TOOL clang)
 
 clang_target_link_libraries(SampleAnalyzerPlugin PRIVATE
   clangAnalysis
Index: clang/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
===
--- clang/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
+++ clang/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
@@ -3,7 +3,7 @@
   )
 
 set(LLVM_EXPORTED_SYMBOL_FILE 
${CMAKE_CURRENT_SOURCE_DIR}/CheckerOptionHandlingAnalyzerPlugin.exports)
-add_llvm_library(CheckerOptionHandlingAnalyzerPlugin MODULE 
CheckerOptionHandling.cpp PLUGIN_TOOL clang)
+add_llvm_library(CheckerOptionHandlingAnalyzerPlugin MODULE BUILDTREE_ONLY 
CheckerOptionHandling.cpp PLUGIN_TOOL clang)
 
 clang_target_link_libraries(CheckerOptionHandlingAnalyzerPlugin PRIVATE
   clangAnalysis
Index: clang/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
===
--- clang/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
+++ clang/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
@@ -3,7 +3,7 @@
   )
 
 set(LLVM_EXPORTED_SYMBOL_FILE 
${CMAKE_CURRENT_SOURCE_DIR}/CheckerDependencyHandlingAnalyzerPlugin.exports)
-add_llvm_library(CheckerDependencyHandlingAnalyzerPlugin MODULE 
CheckerDependencyHandling.cpp PLUGIN_TOOL clang)
+add_llvm_library(CheckerDependencyHandlingAnalyzerPlugin MODULE BUILDTREE_ONLY 
CheckerDependencyHandling.cpp PLUGIN_TOOL clang)
 
 clang_target_link_libraries(CheckerDependencyHandlingAnalyzerPlugin PRIVATE
   clangAnalysis


Index: clang/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
===
--- clang/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
+++ clang/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
@@ -3,7 +3,7 @@
   )
 
 set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/SampleAnalyzerPlugin.exports)
-add_llvm_library(SampleAnalyzerPlugin MODULE MainCallChecker.cpp PLUGIN_TOOL clang)
+add_llvm_library(SampleAnalyzerPlugin MODULE BUILDTREE_ONLY MainCallChecker.cpp PLUGIN_TOOL clang)
 
 clang_target_link_libraries(SampleAnalyzerPlugin PRIVATE
   clangAnalysis
Index: clang/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
===
--- clang/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
+++ clang/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
@@ -3,7 +3,7 @@
   )
 
 set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/CheckerOptionHandlingAnalyzerPlugin.exports)
-add_llvm_library(CheckerOptionHandlingAnalyzerPlugin MODULE CheckerOptionHandling.cpp PLUGIN_TOOL clang)
+add_llvm_library(CheckerOptionHandlingAnalyzerPlugin MODULE BUILDTREE_ONLY CheckerOptionHandling.cpp PLUGIN_TOOL clang)
 
 clang_target_link_libraries(CheckerOptionHandlingAnalyzerPlugin PRIVATE
   clangAnalysis
Index: clang/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
===
--- clang/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
+++ clang/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
@@ -3,7 +3,7 @@
   )
 
 set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/CheckerDependencyHandlingAnalyzerPlugin.exports)
-add_llvm_library(CheckerDependencyHandlingAnalyzerPlugin MODULE CheckerDependencyHandling.cpp PLUGIN_TOOL clang)
+add_llvm_library(CheckerDependencyHandlingAnalyzerPlugin MODULE BUILDTREE_ONLY CheckerDependencyHandling.cpp PLUGIN_TOOL clang)
 
 clang_target_link_libraries(CheckerDependencyHandlingAnalyzerPlugin PRIVATE
   clangAnalysis
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Found a use case where `getCanonicalType()` causes problems:

  @interface NSObject
  @end
  
  @protocol SomeProtocol
  @end
  
  @interface NSString : NSObject
  @end
  @interface NSNumber : NSObject
  @end
  
  @interface Container
  @end
  
  @interface Container (extension)
  T FunctionInExtension(void);
  @end
  
  void test(Container *c) {
NSNumber *n = FunctionInExtension();
  }

In this case we don't warn about `NSNumber`/`NSString` mismatch but should. At 
line 8125 

 in `Sema::CheckAssignmentConstraints` we get a canonical type for the 
right-hand side and it turned out to be `id`, which is compatible with 
`NSNumber`.

I believe it's a separate issue and the currently proposed change is correct 
but insufficient. Prior to using `ObjCTypeParamType` for generic parameters, we 
were using `TypedefType` and I think we need to make `ObjCTypeParamType` like 
`TypedefType` in more places.


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

https://reviews.llvm.org/D66696



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

[PATCH] D68115: Zero initialize padding in unions

[Vitaly Buka via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGen/init.c:197
   // CHECK-LABEL: @nonzeroPaddedUnionMemset(
-  // CHECK-NOT: store
-  // CHECK-NOT: memcpy
-  // CHECK: call void @llvm.memset.p0i8.i32(i8* {{.*}}, i8 -16, i32 36, i1 
false)
+  // CHECK: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 {{.*}}, i8* align 
4 {{.*}} [[INIT_PADDEDUNION]], {{.*}}, i32 36, i1 false)
 }

hubert.reinterpretcast wrote:
> vitalybuka wrote:
> > vitalybuka wrote:
> > > hubert.reinterpretcast wrote:
> > > > This is C++ aggregate initialization and not value-initialization. The 
> > > > wording you quoted from the C++ standard is for zero-initialization, 
> > > > which might be part of value initialization, but you have not shown 
> > > > that aggregate initialization of a union involves zero-initialization 
> > > > of that union.
> > > reading this more I don't see any evidence that either C++ or C requires 
> > > padding initialization.
> > > Reading this I expect that all function here should be equivalent 
> > > https://godbolt.org/z/1O_9-e
> > > But they are not. Clang and GCC initialized padding after the first 
> > > member.
> > So if go trough "aggregates" then nothing is said about padding in union.
> > If we go trough "list-initialization" then value initialization should be 
> > applied, part of which is zero initialization. 
> > If so union padding should be initialized.
> > 
> > 
> In C++14, the list in [dcl.init.list] starts with:
> If T is an aggregate, aggregate initialization is performed.
> 
> The bullet in C++17 follows only a case for copying and then a case for 
> string literals.
> 
> The bullet in the C++2a CD likewise, with an additional earlier bullet that 
> also becomes aggregate initialization.
> 
> It is in C++11 where an empty list gets value-initialization treatment, and 
> the next bullet goes to aggregate initialization.
> 
> The inline comment was not added to an empty list case.
So if I understand you (and what I see in C++17 and C++11) then only C++11 will 
essentially requires zeros via value-initialization.
And the rest (including C), requires only the first field to be initialized and 
the tail is undefined.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68115



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

[PATCH] D68115: Zero initialize padding in unions

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/init.c:197
   // CHECK-LABEL: @nonzeroPaddedUnionMemset(
-  // CHECK-NOT: store
-  // CHECK-NOT: memcpy
-  // CHECK: call void @llvm.memset.p0i8.i32(i8* {{.*}}, i8 -16, i32 36, i1 
false)
+  // CHECK: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 {{.*}}, i8* align 
4 {{.*}} [[INIT_PADDEDUNION]], {{.*}}, i32 36, i1 false)
 }

vitalybuka wrote:
> vitalybuka wrote:
> > hubert.reinterpretcast wrote:
> > > This is C++ aggregate initialization and not value-initialization. The 
> > > wording you quoted from the C++ standard is for zero-initialization, 
> > > which might be part of value initialization, but you have not shown that 
> > > aggregate initialization of a union involves zero-initialization of that 
> > > union.
> > reading this more I don't see any evidence that either C++ or C requires 
> > padding initialization.
> > Reading this I expect that all function here should be equivalent 
> > https://godbolt.org/z/1O_9-e
> > But they are not. Clang and GCC initialized padding after the first member.
> So if go trough "aggregates" then nothing is said about padding in union.
> If we go trough "list-initialization" then value initialization should be 
> applied, part of which is zero initialization. 
> If so union padding should be initialized.
> 
> 
In C++14, the list in [dcl.init.list] starts with:
If T is an aggregate, aggregate initialization is performed.

The bullet in C++17 follows only a case for copying and then a case for string 
literals.

The bullet in the C++2a CD likewise, with an additional earlier bullet that 
also becomes aggregate initialization.

It is in C++11 where an empty list gets value-initialization treatment, and the 
next bullet goes to aggregate initialization.

The inline comment was not added to an empty list case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68115



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

[PATCH] D68115: Zero initialize padding in unions

[Vitaly Buka via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGen/init.c:197
   // CHECK-LABEL: @nonzeroPaddedUnionMemset(
-  // CHECK-NOT: store
-  // CHECK-NOT: memcpy
-  // CHECK: call void @llvm.memset.p0i8.i32(i8* {{.*}}, i8 -16, i32 36, i1 
false)
+  // CHECK: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 {{.*}}, i8* align 
4 {{.*}} [[INIT_PADDEDUNION]], {{.*}}, i32 36, i1 false)
 }

vitalybuka wrote:
> hubert.reinterpretcast wrote:
> > This is C++ aggregate initialization and not value-initialization. The 
> > wording you quoted from the C++ standard is for zero-initialization, which 
> > might be part of value initialization, but you have not shown that 
> > aggregate initialization of a union involves zero-initialization of that 
> > union.
> reading this more I don't see any evidence that either C++ or C requires 
> padding initialization.
> Reading this I expect that all function here should be equivalent 
> https://godbolt.org/z/1O_9-e
> But they are not. Clang and GCC initialized padding after the first member.
So if go trough "aggregates" then nothing is said about padding in union.
If we go trough "list-initialization" then value initialization should be 
applied, part of which is zero initialization. 
If so union padding should be initialized.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68115



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

[PATCH] D68115: Zero initialize padding in unions

[Vitaly Buka via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGen/init.c:197
   // CHECK-LABEL: @nonzeroPaddedUnionMemset(
-  // CHECK-NOT: store
-  // CHECK-NOT: memcpy
-  // CHECK: call void @llvm.memset.p0i8.i32(i8* {{.*}}, i8 -16, i32 36, i1 
false)
+  // CHECK: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 {{.*}}, i8* align 
4 {{.*}} [[INIT_PADDEDUNION]], {{.*}}, i32 36, i1 false)
 }

hubert.reinterpretcast wrote:
> This is C++ aggregate initialization and not value-initialization. The 
> wording you quoted from the C++ standard is for zero-initialization, which 
> might be part of value initialization, but you have not shown that aggregate 
> initialization of a union involves zero-initialization of that union.
reading this more I don't see any evidence that either C++ or C requires 
padding initialization.
Reading this I expect that all function here should be equivalent 
https://godbolt.org/z/1O_9-e
But they are not. Clang and GCC initialized padding after the first member.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68115



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

[PATCH] D67592: [Clang] Use -main-file-name for source filename if not set

[Teresa Johnson via Phabricator via cfe-commits]

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

lgtm


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

https://reviews.llvm.org/D67592



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

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

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev added a comment.

The second part was uploaded to https://reviews.llvm.org/D68166.


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

https://reviews.llvm.org/D64943



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

[PATCH] D68161: [TimeProfiler] Fix "OptModule" section and add new "Backend" sections

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri accepted this revision.
lebedev.ri added a reviewer: thakis.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Oh nice!
F10096481: image.png 
F10096488: image.png 
I like this.

One more related question - there are two `"Frontend"` sections, second one 
appears to consist only of `"CodeGen Function"s`.
Is that correct? Should they both be in an actual front-end section, and should 
they be renamed to, i dunno,
`"Sema"` and `"Codegen"`? I'm not sure about this, just raising a question.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68161



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

[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

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

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



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:890
 
-bool MemFunctionInfoTy::isCMemAllocFunction(const CallEvent ) const {
-  if (Call.isCalled(CD_malloc, CD_realloc, CD_reallocf, CD_calloc, CD_valloc,
-CD_strdup, CD_win_strdup, CD_strndup, CD_wcsdup,
-CD_win_wcsdup, CD_kmalloc, CD_g_malloc, CD_g_malloc0,
-CD_g_realloc, CD_g_try_malloc, CD_g_try_malloc0,
-CD_g_try_realloc, CD_g_memdup, CD_g_malloc_n,
-CD_g_malloc0_n, CD_g_realloc_n, CD_g_try_malloc_n,
-CD_g_try_malloc0_n, CD_g_try_realloc_n))
-return true;
-
-  if (Call.isCalled(CD_if_nameindex))
-return true;
-
-  if (Call.isCalled(CD_alloca, CD_win_alloca))
+bool MallocChecker::isMemCall(const CallEvent ) const {
+  if (FreeingMemFnMap.lookup(Call) || NonFreeingMemFnMap.lookup(Call))

It may be more precise to call this `isCMemCall`, because that's the question 
it answers.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1487
 
-  if (Att->getModule()->getName() !=
-  MemFunctionInfo.CD_malloc.getFunctionName())
+  if (Att->getModule()->getName() != "malloc")
 return nullptr;

Ugh, no idea whats happening here, I'll be honest. I sweat to dig a bit deeper 
to find out, though I didn't want to bother delaying the publishing this patch 
for a feature not documented, or used anywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68165



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

[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

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

Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, Charusso, 
baloghadamsoftware, dcoughlin.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.
Szelethus added a parent revision: D68163: [analyzer][MallocChecker][NFC] 
Change the use of IdentifierInfo* to CallDescription.

Since its important to know whether a function frees memory (even if its a 
reallocating function!), I used two `CallDescriptionMap`s to merge all 
`CallDescription`s into it. `MemFunctionInfoTy` no longer makes sense, it may 
never have, but for now, it would be more of a distraction then anything else.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68165

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -60,6 +60,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -91,8 +92,6 @@
   AF_InnerBuffer
 };
 
-struct MemFunctionInfoTy;
-
 } // end of anonymous namespace
 
 /// Print names of allocators and deallocators.
@@ -265,45 +264,6 @@
 // Kinds of memory operations, information about resource managing functions.
 //===--===//
 
-namespace {
-
-struct MemFunctionInfoTy {
-  /// The value of the MallocChecker:Optimistic is stored in this variable.
-  ///
-  /// In pessimistic mode, the checker assumes that it does not know which
-  /// functions might free the memory.
-  /// In optimistic mode, the checker assumes that all user-defined functions
-  /// which might free a pointer are annotated.
-  DefaultBool ShouldIncludeOwnershipAnnotatedFunctions;
-
-  CallDescription CD_alloca{{"alloca"}, 1}, CD_win_alloca{{"_alloca"}, 1},
-  CD_malloc{{"malloc"}, 1}, CD_free{{"free"}, 1},
-  CD_realloc{{"realloc"}, 2}, CD_calloc{{"calloc"}, 2},
-  CD_valloc{{"valloc"}, 1}, CD_reallocf{{"reallocf"}, 2},
-  CD_strndup{{"strndup"}, 2}, CD_strdup{{"strdup"}, 1},
-  CD_win_strdup{{"_strdup"}, 1}, CD_kmalloc{{"kmalloc"}, 2},
-  CD_if_nameindex{{"if_nameindex"}, 1},
-  CD_if_freenameindex{{"if_freenameindex"}, 1}, CD_wcsdup{{"wcsdup"}, 1},
-  CD_win_wcsdup{{"_wcsdup"}, 1}, CD_kfree{{"kfree"}, 2},
-  CD_g_malloc{{"g_malloc"}, 1}, CD_g_malloc0{{"g_malloc0"}, 1},
-  CD_g_realloc{{"g_realloc"}, 2}, CD_g_try_malloc{{"g_try_malloc"}, 1},
-  CD_g_try_malloc0{{"g_try_malloc0"}, 1},
-  CD_g_try_realloc{{"g_try_realloc"}, 2}, CD_g_free{{"g_free"}, 1},
-  CD_g_memdup{{"g_memdup"}, 2}, CD_g_malloc_n{{"g_malloc_n"}, 2},
-  CD_g_malloc0_n{{"g_malloc0_n"}, 2}, CD_g_realloc_n{{"g_realloc_n"}, 3},
-  CD_g_try_malloc_n{{"g_try_malloc_n"}, 2},
-  CD_g_try_malloc0_n{{"g_try_malloc0_n"}, 2},
-  CD_g_try_realloc_n{{"g_try_realloc_n"}, 3};
-
-  void initIdentifierInfo(ASTContext ) const;
-
-  bool isMemFunction(const CallEvent ) const;
-  bool isCMemFunction(const CallEvent ) const;
-  bool isCMemFreeFunction(const CallEvent ) const;
-  bool isCMemAllocFunction(const CallEvent ) const;
-};
-} // end of anonymous namespace
-
 /// Tells if the callee is one of the builtin new/delete operators, including
 /// placement operators and other standard overloads.
 static bool isStandardNewDelete(const FunctionDecl *FD);
@@ -327,7 +287,11 @@
  check::PreStmt, check::PostStmt,
  check::PostObjCMessage, check::Location, eval::Assume> {
 public:
-  MemFunctionInfoTy MemFunctionInfo;
+  /// In pessimistic mode, the checker assumes that it does not know which
+  /// functions might free the memory.
+  /// In optimistic mode, the checker assumes that all user-defined functions
+  /// which might free a pointer are annotated.
+  DefaultBool ShouldIncludeOwnershipAnnotatedFunctions;
 
   /// Many checkers are essentially built into this one, so enabling them will
   /// make MallocChecker perform additional modeling and reporting.
@@ -387,6 +351,74 @@
   mutable std::unique_ptr BT_OffsetFree[CK_NumCheckKinds];
   mutable std::unique_ptr BT_UseZerroAllocated[CK_NumCheckKinds];
 
+#define CHECK_FN(NAME) \
+  void NAME(CheckerContext , const CallExpr *CE, ProgramStateRef State) const;
+
+  template 
+  void checkRealloc(CheckerContext , const CallExpr *CE,
+ProgramStateRef State) 

[PATCH] D68163: [analyzer][MallocChecker][NFC] Change the use of IdentifierInfo* to CallDescription

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

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

Exactly what it says on the tin! I decided not to merge this with the patch 
that changes all these to a `CallDescriptionMap` object, so the patch is that 
much more trivial.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68163

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -267,8 +267,6 @@
 
 namespace {
 
-enum class MemoryOperationKind { MOK_Allocate, MOK_Free, MOK_Any };
-
 struct MemFunctionInfoTy {
   /// The value of the MallocChecker:Optimistic is stored in this variable.
   ///
@@ -278,44 +276,43 @@
   /// which might free a pointer are annotated.
   DefaultBool ShouldIncludeOwnershipAnnotatedFunctions;
 
-  // TODO: Change these to CallDescription, and get rid of lazy initialization.
-  mutable IdentifierInfo *II_alloca = nullptr, *II_win_alloca = nullptr,
- *II_malloc = nullptr, *II_free = nullptr,
- *II_realloc = nullptr, *II_calloc = nullptr,
- *II_valloc = nullptr, *II_reallocf = nullptr,
- *II_strndup = nullptr, *II_strdup = nullptr,
- *II_win_strdup = nullptr, *II_kmalloc = nullptr,
- *II_if_nameindex = nullptr,
- *II_if_freenameindex = nullptr, *II_wcsdup = nullptr,
- *II_win_wcsdup = nullptr, *II_g_malloc = nullptr,
- *II_g_malloc0 = nullptr, *II_g_realloc = nullptr,
- *II_g_try_malloc = nullptr,
- *II_g_try_malloc0 = nullptr,
- *II_g_try_realloc = nullptr, *II_g_free = nullptr,
- *II_g_memdup = nullptr, *II_g_malloc_n = nullptr,
- *II_g_malloc0_n = nullptr, *II_g_realloc_n = nullptr,
- *II_g_try_malloc_n = nullptr,
- *II_g_try_malloc0_n = nullptr, *II_kfree = nullptr,
- *II_g_try_realloc_n = nullptr;
+  CallDescription CD_alloca{{"alloca"}, 1}, CD_win_alloca{{"_alloca"}, 1},
+  CD_malloc{{"malloc"}, 1}, CD_free{{"free"}, 1},
+  CD_realloc{{"realloc"}, 2}, CD_calloc{{"calloc"}, 2},
+  CD_valloc{{"valloc"}, 1}, CD_reallocf{{"reallocf"}, 2},
+  CD_strndup{{"strndup"}, 2}, CD_strdup{{"strdup"}, 1},
+  CD_win_strdup{{"_strdup"}, 1}, CD_kmalloc{{"kmalloc"}, 2},
+  CD_if_nameindex{{"if_nameindex"}, 1},
+  CD_if_freenameindex{{"if_freenameindex"}, 1}, CD_wcsdup{{"wcsdup"}, 1},
+  CD_win_wcsdup{{"_wcsdup"}, 1}, CD_kfree{{"kfree"}, 2},
+  CD_g_malloc{{"g_malloc"}, 1}, CD_g_malloc0{{"g_malloc0"}, 1},
+  CD_g_realloc{{"g_realloc"}, 2}, CD_g_try_malloc{{"g_try_malloc"}, 1},
+  CD_g_try_malloc0{{"g_try_malloc0"}, 1},
+  CD_g_try_realloc{{"g_try_realloc"}, 2}, CD_g_free{{"g_free"}, 1},
+  CD_g_memdup{{"g_memdup"}, 2}, CD_g_malloc_n{{"g_malloc_n"}, 2},
+  CD_g_malloc0_n{{"g_malloc0_n"}, 2}, CD_g_realloc_n{{"g_realloc_n"}, 3},
+  CD_g_try_malloc_n{{"g_try_malloc_n"}, 2},
+  CD_g_try_malloc0_n{{"g_try_malloc0_n"}, 2},
+  CD_g_try_realloc_n{{"g_try_realloc_n"}, 3};
 
   void initIdentifierInfo(ASTContext ) const;
 
-  ///@{
-  /// Check if this is one of the functions which can allocate/reallocate
-  /// memory pointed to by one of its arguments.
-  bool isMemFunction(const FunctionDecl *FD, ASTContext ) const;
-  bool isCMemFunction(const FunctionDecl *FD, ASTContext ,
-  AllocationFamily Family,
-  MemoryOperationKind MemKind) const;
-
-  /// Tells if the callee is one of the builtin new/delete operators, including
-  /// placement operators and other standard overloads.
-  bool isStandardNewDelete(const FunctionDecl *FD, ASTContext ) const;
-  ///@}
+  bool isMemFunction(const CallEvent ) const;
+  bool isCMemFunction(const CallEvent ) const;
+  bool isCMemFreeFunction(const CallEvent ) const;
+  bool isCMemAllocFunction(const CallEvent ) const;
 };
-
 } // end of anonymous namespace
 
+/// Tells if the callee is one of the builtin new/delete operators, including
+/// placement operators and other standard overloads.
+static bool isStandardNewDelete(const FunctionDecl *FD);
+static bool isStandardNewDelete(const CallEvent ) {
+  if (!Call.getDecl())
+return false;
+  return isStandardNewDelete(cast(Call.getDecl()));
+}
+
 

[PATCH] D68162: [analyzer][MallocChecker][NFC] Communicate the allocation family information to auxiliary functions with template parameters

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

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

The following series of refactoring patches aim to fix the horrible mess that 
MallocChecker.cpp is.

I genuinely hate this file. It goes completely against how most of the checkers 
are implemented, its by far the biggest headache regarding checker 
dependencies, checker options, or anything you can imagine. On top of all that, 
its just bad code. Its seriously everything that you shouldn't do in C++, or 
any other language really. Bad variable/class names, in/out parameters... 
Apologies, rant over.

So: there are a variety of memory manipulating function this checker models. 
One aspect of these functions is their `AllocationFamily`, which we use to 
distinguish between allocation kinds, like using `free()` on an object 
allocated by `operator new`. However, since we always know which function we're 
actually modeling, in fact we know it //compile time//, there is no need to use 
tricks to retrieve this information out of thin air n+1 function calls down the 
line. This patch changes many methods of `MallocChecker` to take a non-optional 
`AllocationFamily` template parameter (which also makes stack dumps a bit 
nicer!), and removes some no longer needed auxiliary functions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68162

Files:
  clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -44,13 +44,14 @@
 //
 //===--===//
 
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "AllocationState.h"
 #include "InterCheckerAPI.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -63,7 +64,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
-#include "AllocationState.h"
+#include "llvm/Support/ErrorHandling.h"
 #include 
 #include 
 
@@ -71,7 +72,10 @@
 using namespace ento;
 
 //===--===//
-// The types of allocation we're modeling.
+// The types of allocation we're modeling. This is used to check whether a
+// dynamically allocated object is deallocated with the correct function, like
+// not using operator delete on an object created by malloc(), or alloca regions
+// aren't ever deallocated manually.
 //===--===//
 
 namespace {
@@ -91,22 +95,14 @@
 
 } // end of anonymous namespace
 
-/// Determine family of a deallocation expression.
-static AllocationFamily
-getAllocationFamily(const MemFunctionInfoTy , CheckerContext ,
-const Stmt *S);
-
 /// Print names of allocators and deallocators.
 ///
 /// \returns true on success.
-static bool printAllocDeallocName(raw_ostream , CheckerContext ,
-  const Expr *E);
+static bool printMemFnName(raw_ostream , CheckerContext , const Expr *E);
 
-/// Print expected name of an allocator based on the deallocator's
-/// family derived from the DeallocExpr.
-static void printExpectedAllocName(raw_ostream ,
-   const MemFunctionInfoTy ,
-   CheckerContext , const Expr *E);
+/// Print expected name of an allocator based on the deallocator's family
+/// derived from the DeallocExpr.
+static void printExpectedAllocName(raw_ostream , AllocationFamily Family);
 
 /// Print expected name of a deallocator based on the allocator's
 /// family.
@@ -205,9 +201,9 @@
 /// Update the RefState to reflect the new memory allocation.
 /// The optional \p RetVal parameter specifies the newly allocated pointer
 /// value; if unspecified, the value of expression \p E is used.
+template 
 static ProgramStateRef MallocUpdateRefState(CheckerContext , const Expr *E,
 ProgramStateRef State,
-AllocationFamily Family = AF_Malloc,
 Optional RetVal = None);
 
 

[PATCH] D68148: [clang-tidy] Rename objc-avoid-spinlock check to darwin-avoid-spinlock

[Michael Wyman via Phabricator via cfe-commits]

mwyman updated this revision to Diff 53.
mwyman added a comment.

Moved release note below list of new checks.


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

https://reviews.llvm.org/D68148

Files:
  clang-tools-extra/clang-tidy/darwin/AvoidSpinlockCheck.cpp
  clang-tools-extra/clang-tidy/darwin/AvoidSpinlockCheck.h
  clang-tools-extra/clang-tidy/darwin/CMakeLists.txt
  clang-tools-extra/clang-tidy/darwin/DarwinTidyModule.cpp
  clang-tools-extra/clang-tidy/objc/AvoidSpinlockCheck.cpp
  clang-tools-extra/clang-tidy/objc/AvoidSpinlockCheck.h
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/darwin-avoid-spinlock.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-avoid-spinlock.rst
  clang-tools-extra/test/clang-tidy/darwin-avoid-spinlock.m
  clang-tools-extra/test/clang-tidy/objc-avoid-spinlock.m

Index: clang-tools-extra/test/clang-tidy/darwin-avoid-spinlock.m
===
--- clang-tools-extra/test/clang-tidy/darwin-avoid-spinlock.m
+++ clang-tools-extra/test/clang-tidy/darwin-avoid-spinlock.m
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s objc-avoid-spinlock %t
+// RUN: %check_clang_tidy %s darwin-avoid-spinlock %t
 
 typedef int OSSpinLock;
 
@@ -6,10 +6,10 @@
 - (void)f {
 int i = 1;
 OSSpinlockLock();
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use os_unfair_lock_lock() or dispatch queue APIs instead of the deprecated OSSpinLock [objc-avoid-spinlock]
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use os_unfair_lock_lock() or dispatch queue APIs instead of the deprecated OSSpinLock [darwin-avoid-spinlock]
 OSSpinlockTry();
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use os_unfair_lock_lock() or dispatch queue APIs instead of the deprecated OSSpinLock [objc-avoid-spinlock]
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use os_unfair_lock_lock() or dispatch queue APIs instead of the deprecated OSSpinLock [darwin-avoid-spinlock]
 OSSpinlockUnlock();
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use os_unfair_lock_lock() or dispatch queue APIs instead of the deprecated OSSpinLock [objc-avoid-spinlock]
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use os_unfair_lock_lock() or dispatch queue APIs instead of the deprecated OSSpinLock [darwin-avoid-spinlock]
 }
 @end
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -212,6 +212,7 @@
cppcoreguidelines-pro-type-vararg
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
+   darwin-avoid-spinlock
darwin-dispatch-once-nonstatic
fuchsia-default-arguments-calls
fuchsia-default-arguments-declarations
@@ -325,7 +326,6 @@
mpi-buffer-deref
mpi-type-mismatch
objc-avoid-nserror-init
-   objc-avoid-spinlock
objc-forbidden-subclassing
objc-missing-hash
objc-property-declaration
Index: clang-tools-extra/docs/clang-tidy/checks/darwin-avoid-spinlock.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/darwin-avoid-spinlock.rst
+++ clang-tools-extra/docs/clang-tidy/checks/darwin-avoid-spinlock.rst
@@ -1,7 +1,7 @@
-.. title:: clang-tidy - objc-avoid-spinlock
+.. title:: clang-tidy - darwin-avoid-spinlock
 
-objc-avoid-spinlock
-===
+darwin-avoid-spinlock
+=
 
 Finds usages of ``OSSpinlock``, which is deprecated due to potential livelock
 problems. 
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,9 @@
   Now also checks if any calls to ``pthread_*`` functions expect negative return
   values.
 
+- The 'objc-avoid-spinlock' check was renamed to :doc:`darwin-avoid-spinlock
+  `
+
 Improvements to include-fixer
 -
 
Index: clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
+++ clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
@@ -10,7 +10,6 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "AvoidNSErrorInitCheck.h"
-#include "AvoidSpinlockCheck.h"
 #include "ForbiddenSubclassingCheck.h"
 #include "MissingHashCheck.h"
 #include "PropertyDeclarationCheck.h"
@@ -27,8 +26,6 @@
   void addCheckFactories(ClangTidyCheckFactories ) override {
 CheckFactories.registerCheck(
 "objc-avoid-nserror-init");
-CheckFactories.registerCheck(
-"objc-avoid-spinlock");
 

[PATCH] D67592: [Clang] Use -main-file-name for source filename if not set

[Joel Klinghed via Phabricator via cfe-commits]

the_jk updated this revision to Diff 51.
the_jk added a comment.

Moved ModuleName change to CodeGeneratorImpl as suggested, agree that it seems 
to fit well there.


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

https://reviews.llvm.org/D67592

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/lib/CodeGen/ModuleBuilder.cpp
  clang/test/Frontend/stdin-input


Index: clang/test/Frontend/stdin-input
===
--- /dev/null
+++ clang/test/Frontend/stdin-input
@@ -0,0 +1,7 @@
+// RUN: cat %s | %clang -emit-llvm -g -S \
+// RUN: -Xclang -main-file-name -Xclang test/foo.c -x c - -o - | FileCheck %s
+// CHECK: ; ModuleID = 'test/foo.c'
+// CHECK: source_filename = "test/foo.c"
+// CHECK: !1 = !DIFile(filename: "test/foo.c"
+
+int main() {}
Index: clang/lib/CodeGen/ModuleBuilder.cpp
===
--- clang/lib/CodeGen/ModuleBuilder.cpp
+++ clang/lib/CodeGen/ModuleBuilder.cpp
@@ -65,6 +65,13 @@
   private:
 SmallVector DeferredInlineMemberFuncDefs;
 
+static llvm::StringRef ExpandModuleName(llvm::StringRef ModuleName,
+const CodeGenOptions ) {
+  if (ModuleName == "-" && !CGO.MainFileName.empty())
+return CGO.MainFileName;
+  return ModuleName;
+}
+
   public:
 CodeGeneratorImpl(DiagnosticsEngine , llvm::StringRef ModuleName,
   const HeaderSearchOptions ,
@@ -73,7 +80,8 @@
   CoverageSourceInfo *CoverageInfo = nullptr)
 : Diags(diags), Ctx(nullptr), HeaderSearchOpts(HSO),
   PreprocessorOpts(PPO), CodeGenOpts(CGO), HandlingTopLevelDecls(0),
-  CoverageInfo(CoverageInfo), M(new llvm::Module(ModuleName, C)) {
+  CoverageInfo(CoverageInfo),
+  M(new llvm::Module(ExpandModuleName(ModuleName, CGO), C)) {
   C.setDiscardValueNames(CGO.DiscardValueNames);
 }
 
@@ -121,7 +129,7 @@
 llvm::Module *StartModule(llvm::StringRef ModuleName,
   llvm::LLVMContext ) {
   assert(!M && "Replacing existing Module?");
-  M.reset(new llvm::Module(ModuleName, C));
+  M.reset(new llvm::Module(ExpandModuleName(ModuleName, CodeGenOpts), C));
   Initialize(*Ctx);
   return M.get();
 }
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -687,7 +687,7 @@
 def version : Flag<["-"], "version">,
   HelpText<"Print the compiler version">;
 def main_file_name : Separate<["-"], "main-file-name">,
-  HelpText<"Main file name to use for debug info">;
+  HelpText<"Main file name to use for debug info and source if missing">;
 def split_dwarf_output : Separate<["-"], "split-dwarf-output">,
   HelpText<"File name to use for split dwarf debug info output">;
 


Index: clang/test/Frontend/stdin-input
===
--- /dev/null
+++ clang/test/Frontend/stdin-input
@@ -0,0 +1,7 @@
+// RUN: cat %s | %clang -emit-llvm -g -S \
+// RUN: -Xclang -main-file-name -Xclang test/foo.c -x c - -o - | FileCheck %s
+// CHECK: ; ModuleID = 'test/foo.c'
+// CHECK: source_filename = "test/foo.c"
+// CHECK: !1 = !DIFile(filename: "test/foo.c"
+
+int main() {}
Index: clang/lib/CodeGen/ModuleBuilder.cpp
===
--- clang/lib/CodeGen/ModuleBuilder.cpp
+++ clang/lib/CodeGen/ModuleBuilder.cpp
@@ -65,6 +65,13 @@
   private:
 SmallVector DeferredInlineMemberFuncDefs;
 
+static llvm::StringRef ExpandModuleName(llvm::StringRef ModuleName,
+const CodeGenOptions ) {
+  if (ModuleName == "-" && !CGO.MainFileName.empty())
+return CGO.MainFileName;
+  return ModuleName;
+}
+
   public:
 CodeGeneratorImpl(DiagnosticsEngine , llvm::StringRef ModuleName,
   const HeaderSearchOptions ,
@@ -73,7 +80,8 @@
   CoverageSourceInfo *CoverageInfo = nullptr)
 : Diags(diags), Ctx(nullptr), HeaderSearchOpts(HSO),
   PreprocessorOpts(PPO), CodeGenOpts(CGO), HandlingTopLevelDecls(0),
-  CoverageInfo(CoverageInfo), M(new llvm::Module(ModuleName, C)) {
+  CoverageInfo(CoverageInfo),
+  M(new llvm::Module(ExpandModuleName(ModuleName, CGO), C)) {
   C.setDiscardValueNames(CGO.DiscardValueNames);
 }
 
@@ -121,7 +129,7 @@
 llvm::Module *StartModule(llvm::StringRef ModuleName,
   llvm::LLVMContext ) {
   assert(!M && "Replacing existing Module?");
-  M.reset(new llvm::Module(ModuleName, C));
+  M.reset(new llvm::Module(ExpandModuleName(ModuleName, CodeGenOpts), C));
   Initialize(*Ctx);
   return M.get();
 }
Index: clang/include/clang/Driver/CC1Options.td

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev marked an inline comment as done.
anton-afanasyev added inline comments.



Comment at: cfe/trunk/lib/CodeGen/BackendUtil.cpp:1426-1431
   EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M);
 
   if (CGOpts.ExperimentalNewPassManager)
 AsmHelper.EmitAssemblyWithNewPassManager(Action, std::move(OS));
   else
 AsmHelper.EmitAssembly(Action, std::move(OS));

anton-afanasyev wrote:
> lebedev.ri wrote:
> > anton-afanasyev wrote:
> > > lebedev.ri wrote:
> > > > anton-afanasyev wrote:
> > > > > lebedev.ri wrote:
> > > > > > anton-afanasyev wrote:
> > > > > > > anton-afanasyev wrote:
> > > > > > > > lebedev.ri wrote:
> > > > > > > > > This isn't covered by any timer; if you look in 
> > > > > > > > > `BackendUtil.cpp`,
> > > > > > > > > `EmitAssemblyHelper` actually has 
> > > > > > > > > `CodeGenerationTime("codegen", "Code Generation Time")` timer.
> > > > > > > > Thanks, I'm to add it.
> > > > > > > Hmm, I've figured out this isn't needed: such new timer mostly 
> > > > > > > coincides with "Backend" timer (above). Legacy `Timer 
> > > > > > > CodeGenerationTime(...)` is bad example of doing right timing.
> > > > > > "Mostly coincides" may not be the best way to approach fine-grained 
> > > > > > timings, i think? :)
> > > > > > 
> > > > > > I have noticed this because when i looked at the produced time 
> > > > > > flame graph,
> > > > > > there's large section in the end that is covered only by 
> > > > > > `"Backend"` timer,
> > > > > > but nothing else. Now, i'm not going to say whether or not that 
> > > > > > extra section
> > > > > > should or should not be within `"Backend"` timer, but it certainly 
> > > > > > should *also*
> > > > > > be within `"codegen"` timer. Or is there no codegen time spent 
> > > > > > there?
> > > > > > {F10062322}
> > > > > > {F10062316}
> > > > > "Mostly coincides" here means "identical" I believe, the difference 
> > > > > is auxiliary stuff.
> > > > > Please look at `clang::EmitBackendOutput()`, `"Backend"` timer is 
> > > > > outer for `"codegen"` one.
> > > > Then we are talking about different things using the same name.
> > > > There are two distinct codegen steps:
> > > > 1. clang AST -> LLVM IR codegen
> > > > (after that, all the opt passes run)
> > > > 2. LLVM IR -> final assembly. This happens after all the opt middle-end 
> > > > passes.
> > > > 
> > > > Those are *different* codegen's.
> > > Yes, and step 1 is named as "CodeGen Function" whereas step 2 is named 
> > > just "Backend".
> > Aha. So this is what i //expected// to see, apparently: 
> > {F10063128} {F10063119}
> > ```
> > diff --git a/clang/lib/CodeGen/BackendUtil.cpp 
> > b/clang/lib/CodeGen/BackendUtil.cpp
> > index 71ae8fd4fb0..d89d12612f8 100644
> > --- a/clang/lib/CodeGen/BackendUtil.cpp
> > +++ b/clang/lib/CodeGen/BackendUtil.cpp
> > @@ -910,6 +910,7 @@ void EmitAssemblyHelper::EmitAssembly(BackendAction 
> > Action,
> >  
> >{
> >  PrettyStackTraceString CrashInfo("Code generation");
> > +llvm::TimeTraceScope TimeScope("BackendCodegen", StringRef(""));
> >  CodeGenPasses.run(*TheModule);
> >}
> > ```
> > 
> > But that actually brings me to another question - what about 
> > `PrettyStackTraceString CrashInfo("Per-function optimization");`?
> > Should that be wrapped into some section, too?
> > I'm less certain here.
> Ok, you've talked about `Timer CodeGenerationTime`, which corresponds to 
> `Backend` scope.
> As for this `BackendCodegen`, adding this I would prefer to add adjacent 
> `"Per-function optimization"` and `"Per-module optimization passes"` together.
Changes are here: https://reviews.llvm.org/D68161


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675



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

[PATCH] D68161: [TimeProfiler] Fix "OptModule" section and add new "Backend" sections

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev created this revision.
anton-afanasyev added a reviewer: lebedev.ri.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, mehdi_amini.
Herald added projects: clang, LLVM.

Remove unnecessary "OptModule" section. Add "PerFunctionPasses", 
"PerModulePasses" and "CodeGenPasses" sections under "Backend" section.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68161

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/lib/IR/LegacyPassManager.cpp


Index: llvm/lib/IR/LegacyPassManager.cpp
===
--- llvm/lib/IR/LegacyPassManager.cpp
+++ llvm/lib/IR/LegacyPassManager.cpp
@@ -1680,7 +1680,6 @@
 bool FPPassManager::runOnModule(Module ) {
   bool Changed = false;
 
-  llvm::TimeTraceScope TimeScope("OptModule", M.getName());
   for (Function  : M)
 Changed |= runOnFunction(F);
 
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -894,6 +894,7 @@
 
   {
 PrettyStackTraceString CrashInfo("Per-function optimization");
+llvm::TimeTraceScope TimeScope("PerFunctionPasses", StringRef(""));
 
 PerFunctionPasses.doInitialization();
 for (Function  : *TheModule)
@@ -904,11 +905,13 @@
 
   {
 PrettyStackTraceString CrashInfo("Per-module optimization passes");
+llvm::TimeTraceScope TimeScope("PerModulePasses", StringRef(""));
 PerModulePasses.run(*TheModule);
   }
 
   {
 PrettyStackTraceString CrashInfo("Code generation");
+llvm::TimeTraceScope TimeScope("CodeGenPasses", StringRef(""));
 CodeGenPasses.run(*TheModule);
   }
 


Index: llvm/lib/IR/LegacyPassManager.cpp
===
--- llvm/lib/IR/LegacyPassManager.cpp
+++ llvm/lib/IR/LegacyPassManager.cpp
@@ -1680,7 +1680,6 @@
 bool FPPassManager::runOnModule(Module ) {
   bool Changed = false;
 
-  llvm::TimeTraceScope TimeScope("OptModule", M.getName());
   for (Function  : M)
 Changed |= runOnFunction(F);
 
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -894,6 +894,7 @@
 
   {
 PrettyStackTraceString CrashInfo("Per-function optimization");
+llvm::TimeTraceScope TimeScope("PerFunctionPasses", StringRef(""));
 
 PerFunctionPasses.doInitialization();
 for (Function  : *TheModule)
@@ -904,11 +905,13 @@
 
   {
 PrettyStackTraceString CrashInfo("Per-module optimization passes");
+llvm::TimeTraceScope TimeScope("PerModulePasses", StringRef(""));
 PerModulePasses.run(*TheModule);
   }
 
   {
 PrettyStackTraceString CrashInfo("Code generation");
+llvm::TimeTraceScope TimeScope("CodeGenPasses", StringRef(""));
 CodeGenPasses.run(*TheModule);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68148: [clang-tidy] Rename objc-avoid-spinlock check to darwin-avoid-spinlock

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:70
 
+- The 'objc-avoid-spinlock' check was renamed to :doc:`darwin-avoid-spinlock
+  `

Please place this after list of new checks. If there are other renamed checks, 
please keep alphabetical order in that list.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68148



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

[PATCH] D66176: Fix Driver/modules.cpp test to work when build directory name contains '.s'

[Douglas Yung via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66176



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

[PATCH] D67592: [Clang] Use -main-file-name for source filename if not set

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

Please upload patches with context 
(https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)

I'm not terribly familiar with this part of clang, but my concern would be that 
overriding the PresumedInputFile here could have unintended side effects. How 
about simply overriding the source file name on the Module object that 
eventually gets created, if it is "-"? I believe that is in the 
CodeGeneratorImpl constructor, and the CodeGenOptions are available there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67592



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

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 45.
lebedev.ri marked 6 inline comments as done.
lebedev.ri added a comment.

Patch updated - mainly wording changes only.

It isn't clear that for C the rule is actually different from C++,
if it is it does not make much sense and is irrelevant for LLVM IR - there is 
no such UB there,
so i'we deferred wasting hours upon hours on test updates until there's 
definitive answer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67122

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-blacklist.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-when-nullptr-is-defined.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset.c
  clang/test/CodeGen/catch-pointer-overflow-volatile.c
  clang/test/CodeGen/catch-pointer-overflow.c
  clang/test/CodeGen/ubsan-pointer-overflow.m
  clang/test/CodeGenCXX/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.cpp
  compiler-rt/lib/ubsan/ubsan_checks.inc
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/index-overflow.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-constants.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-summary.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
  compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -53,6 +53,20 @@
 
Makes programs 10x faster by doing Special New Thing.
 
+* As per :ref:`LLVM Language Reference Manual `,
+  ``getelementptr inbounds`` can not change the null status of a pointer,
+  meaning it can not produce non-null pointer given null base pointer, and
+  likewise given non-null base pointer it can not produce null pointer; if it
+  does, the result is a :ref:`poison value `.
+  Since `r369789 `_
+  (`D66608 `_ ``[InstCombine] icmp eq/ne (gep
+  inbounds P, Idx..), null -> icmp eq/ne P, null``) LLVM uses that for
+  transformations. If the original source violates these requirements this
+  may result in code being miscompiled. If you are using Clang front-end,
+  Undefined Behaviour Sanitizer ``-fsanitize=pointer-overflow`` check
+  will now catch such cases.
+
+
 Changes to the LLVM IR
 --
 
Index: compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
===
--- /dev/null
+++ compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
@@ -0,0 +1,23 @@
+// RUN: %clang -fsanitize=pointer-overflow %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="pointer-overflow"
+
+#include 
+
+int main(int argc, char *argv[]) {
+  char *base, *result;
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)0;
+  result = base + 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)1;
+  result = base - 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  return 0;
+}
Index: compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
===
--- compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
+++ compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
@@ -1,5 +1,5 @@
 // RUN: %clangxx -std=c++11 -fsanitize=pointer-overflow %s -o %t
-// RUN: %run %t 2>&1 | FileCheck %s
+// RUN: %run %t 2>&1 | FileCheck %s --implicit-check-not="error:"
 
 int main(int argc, char *argv[]) {
   char c;
@@ -12,7 +12,7 @@
   // CHECK: unsigned-index-expression.cpp:[[@LINE+1]]:16: runtime error: subtraction of unsigned offset from 0x{{.*}} overflowed to 0x{{.*}}
   char *q1 = p - neg_1;
 
-  // CHECK: unsigned-index-expression.cpp:[[@LINE+2]]:16: runtime error: pointer index expression with base 0x{{0*}} overflowed to 0x{{.*}}
+  // CHECK: unsigned-index-expression.cpp:[[@LINE+2]]:16: runtime error: applying non-zero offset {{.*}} to null pointer
   char *n = nullptr;
   char *q2 = n - 1ULL;
 
Index: compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang -x c   -fsanitize=pointer-overflow -O0 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" 

[PATCH] D66834: Driver tests: set `--sysroot=""` to support clang with `DEFAULT_SYSROOT`

[Sergej Jaskiewicz via Phabricator via cfe-commits]

broadwaylamb added a comment.

Can someone please commit this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66834



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

r373122 - For P0784R7: add support for explicit destructor calls and

[Richard Smith via cfe-commits]

Author: rsmith
Date: Fri Sep 27 13:24:36 2019
New Revision: 373122

URL: http://llvm.org/viewvc/llvm-project?rev=373122=rev
Log:
For P0784R7: add support for explicit destructor calls and
pseudo-destructor calls in constant evaluation.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/AST/Interp/State.h
cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=373122=373121=373122=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Fri Sep 27 13:24:36 2019
@@ -38,7 +38,8 @@ def note_constexpr_pure_virtual_call : N
   "pure virtual function %q0 called">;
 def note_constexpr_polymorphic_unknown_dynamic_type : Note<
   "%select{|virtual function called on|dynamic_cast applied to|"
-  "typeid applied to}0 object '%1' whose dynamic type is not constant">;
+  "typeid applied to|destruction of}0 object '%1' whose dynamic type "
+  "is not constant">;
 def note_constexpr_dynamic_cast_to_reference_failed : Note<
   "reference dynamic_cast failed: %select{"
   "static type %1 of operand is a non-public base class of dynamic type %2|"
@@ -120,11 +121,11 @@ def note_constexpr_this : Note<
   "evaluation of a call to a 'constexpr' member function">;
 def note_constexpr_lifetime_ended : Note<
   "%select{read of|read of|assignment to|increment of|decrement of|"
-  "member call on|dynamic_cast of|typeid applied to}0 "
+  "member call on|dynamic_cast of|typeid applied to|destruction of}0 "
   "%select{temporary|variable}1 whose lifetime has ended">;
 def note_constexpr_access_uninit : Note<
   "%select{read of|read of|assignment to|increment of|decrement of|"
-  "member call on|dynamic_cast of|typeid applied to}0 "
+  "member call on|dynamic_cast of|typeid applied to|destruction of}0 "
   "%select{object outside its lifetime|uninitialized object}1 "
   "is not allowed in a constant expression">;
 def note_constexpr_use_uninit_reference : Note<
@@ -135,11 +136,11 @@ def note_constexpr_modify_const_type : N
   "in a constant expression">;
 def note_constexpr_access_volatile_type : Note<
   "%select{read of|read of|assignment to|increment of|decrement of|"
-  "|}0 "
+  "||}0 "
   "volatile-qualified type %1 is not allowed in a constant expression">;
 def note_constexpr_access_volatile_obj : Note<
   "%select{read of|read of|assignment to|increment of|decrement of|"
-  "|}0 "
+  "||}0 "
   "volatile %select{temporary|object %2|member %2}1 is not allowed in "
   "a constant expression">;
 def note_constexpr_volatile_here : Note<
@@ -154,36 +155,36 @@ def note_constexpr_ltor_incomplete_type
   "read of incomplete type %0 is not allowed in a constant expression">;
 def note_constexpr_access_null : Note<
   "%select{read of|read of|assignment to|increment of|decrement of|"
-  "member call on|dynamic_cast of|typeid applied to}0 "
+  "member call on|dynamic_cast of|typeid applied to|destruction of}0 "
   "dereferenced null pointer is not allowed in a constant expression">;
 def note_constexpr_access_past_end : Note<
   "%select{read of|read of|assignment to|increment of|decrement of|"
-  "member call on|dynamic_cast of|typeid applied to}0 "
+  "member call on|dynamic_cast of|typeid applied to|destruction of}0 "
   "dereferenced one-past-the-end pointer is not allowed "
   "in a constant expression">;
 def note_constexpr_access_unsized_array : Note<
   "%select{read of|read of|assignment to|increment of|decrement of|"
-  "member call on|dynamic_cast of|typeid applied to}0 "
+  "member call on|dynamic_cast of|typeid applied to|destruction of}0 "
   "element of array without known bound "
   "is not allowed in a constant expression">;
 def note_constexpr_access_inactive_union_member : Note<
   "%select{read of|read of|assignment to|increment of|decrement of|"
-  "member call on|dynamic_cast of|typeid applied to}0 "
+  "member call on|dynamic_cast of|typeid applied to|destruction of}0 "
   "member %1 of union with %select{active member %3|no active member}2 "
   "is not allowed in a constant expression">;
 def note_constexpr_access_static_temporary : Note<
   "%select{read of|read of|assignment to|increment of|decrement of|"
-  "member call on|dynamic_cast of|typeid applied to}0 temporary "
+  "member call on|dynamic_cast of|typeid applied to|destruction of}0 temporary 
"
   "is not allowed in a constant expression outside the expression that "
   "created the temporary">;
 def note_constexpr_access_unreadable_object : Note<
   "%select{read of|read of|assignment to|increment of|decrement of|"
-  "member call on|dynamic_cast of|typeid applied to}0 object '%1' "
-  "whose value is not known">;
+  

[PATCH] D68157: [X86][ABI] Keep empty class argument passing by value compatible with GCC.

[Michael Liao via Phabricator via cfe-commits]

hliao created this revision.
hliao added a reviewer: craig.topper.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68157

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/empty-class.cpp


Index: clang/test/CodeGen/empty-class.cpp
===
--- /dev/null
+++ clang/test/CodeGen/empty-class.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64 | FileCheck 
--check-prefix=X64 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686 | FileCheck 
--check-prefix=X32 %s
+
+class Empty {};
+
+void bar(Empty *);
+
+// X64-LABEL: _Z3foo5Empty
+// X64-SAME: %class.Empty* byval(%class.Empty) align 8 %e
+// X64-NOT: alloca
+// X32-LABEL: _Z3foo5Empty
+// X32-SAME: %class.Empty* byval(%class.Empty) align 4 %e
+// X32-NOT: alloca
+void foo(Empty e) {
+  bar();
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1670,8 +1670,12 @@
   return getIndirectResult(Ty, true, State);
 
 // Ignore empty structs/unions on non-Windows.
-if (!IsWin32StructABI && isEmptyRecord(getContext(), Ty, true))
+if (!IsWin32StructABI && isEmptyRecord(getContext(), Ty, true)) {
+  // For compatibility with GCC, treat it like a struct with a single char.
+  if (getContext().getLangOpts().CPlusPlus)
+return getIndirectResult(Ty, /*ByVal=*/true, State);
   return ABIArgInfo::getIgnore();
+}
 
 llvm::LLVMContext  = getVMContext();
 llvm::IntegerType *Int32 = llvm::Type::getInt32Ty(LLVMContext);
@@ -2799,6 +2803,16 @@
 if (RD->hasFlexibleArrayMember())
   return;
 
+// According to the resolution to A-5 in
+// https://itanium-cxx-abi.github.io/cxx-abi/cxx-open.html, empty class by
+// value should be treated as a struct containing a single character.
+// That's aligned with record layout in AST and compatible with gcc.
+// Check https://godbolt.org/z/D1u2MV for the difference in codegen.
+if (getContext().getLangOpts().CPlusPlus)
+  if (const CXXRecordDecl *CXXRD = dyn_cast(RD))
+if (CXXRD->isEmpty())
+  return;
+
 const ASTRecordLayout  = getContext().getASTRecordLayout(RD);
 
 // Reset Lo class, this will be recomputed.


Index: clang/test/CodeGen/empty-class.cpp
===
--- /dev/null
+++ clang/test/CodeGen/empty-class.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64 | FileCheck --check-prefix=X64 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686 | FileCheck --check-prefix=X32 %s
+
+class Empty {};
+
+void bar(Empty *);
+
+// X64-LABEL: _Z3foo5Empty
+// X64-SAME: %class.Empty* byval(%class.Empty) align 8 %e
+// X64-NOT: alloca
+// X32-LABEL: _Z3foo5Empty
+// X32-SAME: %class.Empty* byval(%class.Empty) align 4 %e
+// X32-NOT: alloca
+void foo(Empty e) {
+  bar();
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1670,8 +1670,12 @@
   return getIndirectResult(Ty, true, State);
 
 // Ignore empty structs/unions on non-Windows.
-if (!IsWin32StructABI && isEmptyRecord(getContext(), Ty, true))
+if (!IsWin32StructABI && isEmptyRecord(getContext(), Ty, true)) {
+  // For compatibility with GCC, treat it like a struct with a single char.
+  if (getContext().getLangOpts().CPlusPlus)
+return getIndirectResult(Ty, /*ByVal=*/true, State);
   return ABIArgInfo::getIgnore();
+}
 
 llvm::LLVMContext  = getVMContext();
 llvm::IntegerType *Int32 = llvm::Type::getInt32Ty(LLVMContext);
@@ -2799,6 +2803,16 @@
 if (RD->hasFlexibleArrayMember())
   return;
 
+// According to the resolution to A-5 in
+// https://itanium-cxx-abi.github.io/cxx-abi/cxx-open.html, empty class by
+// value should be treated as a struct containing a single character.
+// That's aligned with record layout in AST and compatible with gcc.
+// Check https://godbolt.org/z/D1u2MV for the difference in codegen.
+if (getContext().getLangOpts().CPlusPlus)
+  if (const CXXRecordDecl *CXXRD = dyn_cast(RD))
+if (CXXRD->isEmpty())
+  return;
+
 const ASTRecordLayout  = getContext().getASTRecordLayout(RD);
 
 // Reset Lo class, this will be recomputed.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68155: [clang][NFC] Make various uses of Regex const

[Nicolas Guillemot via Phabricator via cfe-commits]

nlguillemot created this revision.
nlguillemot added a reviewer: thopre.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The const-correctness of match() was fixed in rL372764 
, which allows
uses of Regex objects to be const in cases they couldn't be before. This
patch tightens up the const-ness of Regex in various such cases.


Repository:
  rC Clang

https://reviews.llvm.org/D68155

Files:
  include/clang/Tooling/Inclusions/HeaderIncludes.h
  lib/Format/BreakableToken.cpp
  lib/Format/BreakableToken.h
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/UnwrappedLineParser.cpp
  tools/clang-refactor/TestSupport.cpp

Index: tools/clang-refactor/TestSupport.cpp
===
--- tools/clang-refactor/TestSupport.cpp
+++ tools/clang-refactor/TestSupport.cpp
@@ -303,9 +303,10 @@
 
   // See the doc comment for this function for the explanation of this
   // syntax.
-  static Regex RangeRegex("range[[:blank:]]*([[:alpha:]_]*)?[[:blank:]]*=[[:"
-  "blank:]]*(\\+[[:digit:]]+)?[[:blank:]]*(->[[:blank:]"
-  "]*[\\+\\:[:digit:]]+)?");
+  static const Regex RangeRegex(
+  "range[[:blank:]]*([[:alpha:]_]*)?[[:blank:]]*=[[:"
+  "blank:]]*(\\+[[:digit:]]+)?[[:blank:]]*(->[[:blank:]"
+  "]*[\\+\\:[:digit:]]+)?");
 
   std::map> GroupedRanges;
 
@@ -352,7 +353,7 @@
 unsigned EndOffset;
 
 if (!Matches[3].empty()) {
-  static Regex EndLocRegex(
+  static const Regex EndLocRegex(
   "->[[:blank:]]*(\\+[[:digit:]]+):([[:digit:]]+)");
   SmallVector EndLocMatches;
   if (!EndLocRegex.match(Matches[3], )) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2495,9 +2495,10 @@
 
 // Checks if \p FormatTok is a line comment that continues the line comment
 // section on \p Line.
-static bool continuesLineCommentSection(const FormatToken ,
-const UnwrappedLine ,
-llvm::Regex ) {
+static bool
+continuesLineCommentSection(const FormatToken ,
+const UnwrappedLine ,
+const llvm::Regex ) {
   if (Line.Tokens.empty())
 return false;
 
Index: lib/Format/NamespaceEndCommentsFixer.cpp
===
--- lib/Format/NamespaceEndCommentsFixer.cpp
+++ lib/Format/NamespaceEndCommentsFixer.cpp
@@ -92,24 +92,24 @@
 
   // Matches a valid namespace end comment.
   // Valid namespace end comments don't need to be edited.
-  static llvm::Regex *const NamespaceCommentPattern =
-  new llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
-  "namespace( +([a-zA-Z0-9:_]+))?\\.? *(\\*/)?$",
-  llvm::Regex::IgnoreCase);
-  static llvm::Regex *const NamespaceMacroCommentPattern =
-  new llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
-  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*)\\)\\.? *(\\*/)?$",
-  llvm::Regex::IgnoreCase);
+  static const llvm::Regex NamespaceCommentPattern =
+  llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
+  "namespace( +([a-zA-Z0-9:_]+))?\\.? *(\\*/)?$",
+  llvm::Regex::IgnoreCase);
+  static const llvm::Regex NamespaceMacroCommentPattern =
+  llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
+  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*)\\)\\.? *(\\*/)?$",
+  llvm::Regex::IgnoreCase);
 
   SmallVector Groups;
   if (NamespaceTok->is(TT_NamespaceMacro) &&
-  NamespaceMacroCommentPattern->match(Comment->TokenText, )) {
+  NamespaceMacroCommentPattern.match(Comment->TokenText, )) {
 StringRef NamespaceTokenText = Groups.size() > 4 ? Groups[4] : "";
 // The name of the macro must be used.
 if (NamespaceTokenText != NamespaceTok->TokenText)
   return false;
   } else if (NamespaceTok->isNot(tok::kw_namespace) ||
- !NamespaceCommentPattern->match(Comment->TokenText, )) {
+ !NamespaceCommentPattern.match(Comment->TokenText, )) {
 // Comment does not match regex.
 return false;
   }
Index: lib/Format/BreakableToken.h
===
--- lib/Format/BreakableToken.h
+++ lib/Format/BreakableToken.h
@@ -155,7 +155,7 @@
   /// file.
   virtual Split getSplit(unsigned LineIndex, unsigned TailOffset,
  unsigned ColumnLimit, unsigned ContentStartColumn,
- llvm::Regex ) const = 0;
+ const llvm::Regex ) const = 0;
 
   /// Emits the previously retrieved \p Split via \p Whitespaces.
   virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split 

[PATCH] D63640: [clang] Improve Serialization/Imporing/Dumping of APValues

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

rsmith added inline comments.



Comment at: clang/lib/AST/Expr.cpp:319
   case RSK_None:
 return;
   case RSK_Int64:

Can you use `llvm_unreachable` here? (Are there cases where we use `RSK_None` 
and then later find we actually have a value to store into the `ConstantExpr`?)



Comment at: clang/lib/Serialization/ASTReader.cpp:9635
+if (IsExpr) {
+  Base = APValue::LValueBase(ReadExpr(F), CallIndex, Version);
+  ElemTy = Base.get()->getType();

This is problematic.

`ReadExpr` will read a new copy of the expression, creating a distinct object. 
But in the case where we reach this when deserializing (for a 
`MaterializeTemporaryExpr`), we need to refer to the existing 
`MaterializeTemporaryExpr` in the initializer of its lifetime-extending 
declaration. We will also need to serialize the `ASTContext`'s 
`MaterializedTemporaryValues` collection so that the temporaries 
lifetime-extended in a constant initializer get properly handled.

That all sounds very messy, so I think we should reconsider the model that we 
use for lifetime-extended materialized temporaries. As a half-baked idea:

 * When we lifetime-extend a temporary, create a `MaterializedTemporaryDecl` to 
hold its value, and modify `MaterializeTemporaryExpr` to refer to the 
`MaterializedTemporaryDecl` rather than to just hold the subexpression for the 
temporary.
 * Change the `LValueBase` representation to denote the declaration rather than 
the expression.
 * Store the constant evaluated value for a materialized temporary on the 
`MaterializedTemporaryDecl` rather than on a side-table in the `ASTContext`.

With that done, we should verify that all remaining `Expr*`s used as 
`LValueBase`s are either only transiently used during evaluation or don't have 
these kinds of identity problems.


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

https://reviews.llvm.org/D63640



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

[PATCH] D64319: [OpenCL] Add function attributes handling for builtin functions

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!


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

https://reviews.llvm.org/D64319



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

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/lib/AST/DeclTemplate.cpp:513
getDefaultArgumentInfo()->getTypeLoc().getEndLoc());
-  else
-return TypeDecl::getSourceRange();
+  else if(getName().empty())
+return SourceRange(getBeginLoc());

Could you provide more details why we need this change?



Comment at: clang/lib/Parse/ParseTemplate.cpp:633
   // Grab the template parameter name (if given)
-  SourceLocation NameLoc;
+  SourceLocation NameLoc = Tok.getLocation();
   IdentifierInfo *ParamName = nullptr;

What happens if there's a comment? E.g.
```
template 
```

where would we point to and does that align with the function parameter case?



Comment at: clang/lib/Sema/SemaTemplate.cpp:1008
 
   SourceLocation Loc = ParamNameLoc;
-  if (!ParamName)

NIT: maybe remove the extra variable and use `ParamNameLoc` everywhere?



Comment at: clang/test/AST/ast-dump-record-definition-data-json.cpp:398
 // CHECK-NEXT:  "range": {
-// CHECK-NEXT:   "begin": {
-// CHECK-NEXT:"col": 29,
-// CHECK-NEXT:"tokLen": 4
-// CHECK-NEXT:   },
-// CHECK-NEXT:   "end": {
-// CHECK-NEXT:"col": 29,
-// CHECK-NEXT:"tokLen": 4
-// CHECK-NEXT:   }
+// CHECK-NEXT:   "begin": {},
+// CHECK-NEXT:   "end": {}

Does that mean we do not have locations in some cases now?
Is that a regression?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68143



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

[PATCH] D68148: [clang-tidy] Rename objc-avoid-spinlock check to darwin-avoid-spinlock

[Michael Wyman via Phabricator via cfe-commits]

mwyman created this revision.
mwyman added reviewers: stephanemoore, dmaclach.
mwyman added projects: clang-tools-extra, clang, LLVM.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
mwyman edited the summary of this revision.

OSSpinLock* are Apple/Darwin functions, but were previously located with ObjC 
checks as those were most closely tied to Apple platforms before.

Now that there's a specific Darwin module, relocating the check there.

This change was prepared by running rename_check.py.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D68148

Files:
  clang-tools-extra/clang-tidy/darwin/AvoidSpinlockCheck.cpp
  clang-tools-extra/clang-tidy/darwin/AvoidSpinlockCheck.h
  clang-tools-extra/clang-tidy/darwin/CMakeLists.txt
  clang-tools-extra/clang-tidy/darwin/DarwinTidyModule.cpp
  clang-tools-extra/clang-tidy/objc/AvoidSpinlockCheck.cpp
  clang-tools-extra/clang-tidy/objc/AvoidSpinlockCheck.h
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/darwin-avoid-spinlock.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-avoid-spinlock.rst
  clang-tools-extra/test/clang-tidy/darwin-avoid-spinlock.m
  clang-tools-extra/test/clang-tidy/objc-avoid-spinlock.m

Index: clang-tools-extra/test/clang-tidy/darwin-avoid-spinlock.m
===
--- clang-tools-extra/test/clang-tidy/darwin-avoid-spinlock.m
+++ clang-tools-extra/test/clang-tidy/darwin-avoid-spinlock.m
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s objc-avoid-spinlock %t
+// RUN: %check_clang_tidy %s darwin-avoid-spinlock %t
 
 typedef int OSSpinLock;
 
@@ -6,10 +6,10 @@
 - (void)f {
 int i = 1;
 OSSpinlockLock();
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use os_unfair_lock_lock() or dispatch queue APIs instead of the deprecated OSSpinLock [objc-avoid-spinlock]
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use os_unfair_lock_lock() or dispatch queue APIs instead of the deprecated OSSpinLock [darwin-avoid-spinlock]
 OSSpinlockTry();
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use os_unfair_lock_lock() or dispatch queue APIs instead of the deprecated OSSpinLock [objc-avoid-spinlock]
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use os_unfair_lock_lock() or dispatch queue APIs instead of the deprecated OSSpinLock [darwin-avoid-spinlock]
 OSSpinlockUnlock();
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use os_unfair_lock_lock() or dispatch queue APIs instead of the deprecated OSSpinLock [objc-avoid-spinlock]
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use os_unfair_lock_lock() or dispatch queue APIs instead of the deprecated OSSpinLock [darwin-avoid-spinlock]
 }
 @end
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -212,6 +212,7 @@
cppcoreguidelines-pro-type-vararg
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
+   darwin-avoid-spinlock
darwin-dispatch-once-nonstatic
fuchsia-default-arguments-calls
fuchsia-default-arguments-declarations
@@ -325,7 +326,6 @@
mpi-buffer-deref
mpi-type-mismatch
objc-avoid-nserror-init
-   objc-avoid-spinlock
objc-forbidden-subclassing
objc-missing-hash
objc-property-declaration
Index: clang-tools-extra/docs/clang-tidy/checks/darwin-avoid-spinlock.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/darwin-avoid-spinlock.rst
+++ clang-tools-extra/docs/clang-tidy/checks/darwin-avoid-spinlock.rst
@@ -1,7 +1,7 @@
-.. title:: clang-tidy - objc-avoid-spinlock
+.. title:: clang-tidy - darwin-avoid-spinlock
 
-objc-avoid-spinlock
-===
+darwin-avoid-spinlock
+=
 
 Finds usages of ``OSSpinlock``, which is deprecated due to potential livelock
 problems. 
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,6 +67,9 @@
 Improvements to clang-tidy
 --
 
+- The 'objc-avoid-spinlock' check was renamed to :doc:`darwin-avoid-spinlock
+  `
+
 - New :doc:`bugprone-dynamic-static-initializers
   ` check.
 
Index: clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
+++ clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
@@ -10,7 +10,6 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "AvoidNSErrorInitCheck.h"
-#include "AvoidSpinlockCheck.h"
 #include 

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

This needs a lot more test coverage: The frontend cases aren't all covered by 
frontend checks and neither is the dwarf output.


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

https://reviews.llvm.org/D68117



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

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Thanks, this looks like a good addition!




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1608
 
+  if (CGM.getCodeGenOpts().DwarfVersion >= 5) {
+// DWARF-5 support for, defaulted, deleted member functions

Please try to always upload patches with more context (git diff -U works 
fine). I can't even tell which function this is in otherwise.


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

https://reviews.llvm.org/D68117



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

[PATCH] D68124: [clangd] Fix template type aliases in findExplicitReference

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373104: [clangd] Fix template type aliases in 
findExplicitReference (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68124?vs=04=07#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68124

Files:
  clang-tools-extra/trunk/clangd/FindTarget.cpp
  clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/trunk/clangd/FindTarget.cpp
===
--- clang-tools-extra/trunk/clangd/FindTarget.cpp
+++ clang-tools-extra/trunk/clangd/FindTarget.cpp
@@ -477,13 +477,6 @@
   Ref->Qualifier = L.getQualifierLoc();
 }
 
-void VisitDeducedTemplateSpecializationTypeLoc(
-DeducedTemplateSpecializationTypeLoc L) {
-  Ref = ReferenceLoc{
-  NestedNameSpecifierLoc(), L.getNameLoc(),
-  explicitReferenceTargets(DynTypedNode::create(L.getType()))};
-}
-
 void VisitTagTypeLoc(TagTypeLoc L) {
   Ref =
   ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), 
{L.getDecl()}};
@@ -495,9 +488,25 @@
 }
 
 void VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc L) {
+  // We must ensure template type aliases are included in results if they
+  // were written in the source code, e.g. in
+  //template  using valias = vector;
+  //^valias x;
+  // 'explicitReferenceTargets' will return:
+  //1. valias with mask 'Alias'.
+  //2. 'vector' with mask 'Underlying'.
+  //  we want to return only #1 in this case.
   Ref = ReferenceLoc{
   NestedNameSpecifierLoc(), L.getTemplateNameLoc(),
-  explicitReferenceTargets(DynTypedNode::create(L.getType()))};
+  explicitReferenceTargets(DynTypedNode::create(L.getType()),
+   DeclRelation::Alias)};
+}
+void VisitDeducedTemplateSpecializationTypeLoc(
+DeducedTemplateSpecializationTypeLoc L) {
+  Ref = ReferenceLoc{
+  NestedNameSpecifierLoc(), L.getNameLoc(),
+  explicitReferenceTargets(DynTypedNode::create(L.getType()),
+   DeclRelation::Alias)};
 }
 
 void VisitDependentTemplateSpecializationTypeLoc(
Index: clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp
@@ -615,20 +615,18 @@
 )cpp",
"0: targets = {vector}\n"
"1: targets = {vector}\n"},
-  // FIXME: Fix 'allTargetDecls' to return alias template and 
re-enable.
   // Template type aliases.
-  //   {R"cpp(
-  //   template  struct vector { using value_type = T; };
-  //   template <> struct vector { using value_type = bool; };
-  //   template  using valias = vector;
-  //   void foo() {
-  // $0^valias vi;
-  // $1^valias vb;
-  //   }
-  // )cpp",
-  //"0: targets = {valias}\n"
-  //"1: targets = {valias}\n"},
-
+  {R"cpp(
+template  struct vector { using value_type = T; };
+template <> struct vector { using value_type = bool; };
+template  using valias = vector;
+void foo() {
+  $0^valias vi;
+  $1^valias vb;
+}
+  )cpp",
+   "0: targets = {valias}\n"
+   "1: targets = {valias}\n"},
   // MemberExpr should know their using declaration.
   {R"cpp(
 struct X { void func(int); }


Index: clang-tools-extra/trunk/clangd/FindTarget.cpp
===
--- clang-tools-extra/trunk/clangd/FindTarget.cpp
+++ clang-tools-extra/trunk/clangd/FindTarget.cpp
@@ -477,13 +477,6 @@
   Ref->Qualifier = L.getQualifierLoc();
 }
 
-void VisitDeducedTemplateSpecializationTypeLoc(
-DeducedTemplateSpecializationTypeLoc L) {
-  Ref = ReferenceLoc{
-  NestedNameSpecifierLoc(), L.getNameLoc(),
-  explicitReferenceTargets(DynTypedNode::create(L.getType()))};
-}
-
 void VisitTagTypeLoc(TagTypeLoc L) {
   Ref =
   ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), {L.getDecl()}};
@@ -495,9 +488,25 @@
 }
 
 void VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc L) {
+  // We must ensure template type aliases are included in results if they
+  // were written in the source code, e.g. in
+  //template  using valias = vector;
+  //^valias x;
+  // 'explicitReferenceTargets' will return:
+  //  

[clang-tools-extra] r373104 - [clangd] Fix template type aliases in findExplicitReference

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Sep 27 10:55:46 2019
New Revision: 373104

URL: http://llvm.org/viewvc/llvm-project?rev=373104=rev
Log:
[clangd] Fix template type aliases in findExplicitReference

Reviewers: kadircet

Reviewed By: kadircet

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/FindTarget.cpp
clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp

Modified: clang-tools-extra/trunk/clangd/FindTarget.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FindTarget.cpp?rev=373104=373103=373104=diff
==
--- clang-tools-extra/trunk/clangd/FindTarget.cpp (original)
+++ clang-tools-extra/trunk/clangd/FindTarget.cpp Fri Sep 27 10:55:46 2019
@@ -477,13 +477,6 @@ Optional refInTypeLoc(Type
   Ref->Qualifier = L.getQualifierLoc();
 }
 
-void VisitDeducedTemplateSpecializationTypeLoc(
-DeducedTemplateSpecializationTypeLoc L) {
-  Ref = ReferenceLoc{
-  NestedNameSpecifierLoc(), L.getNameLoc(),
-  explicitReferenceTargets(DynTypedNode::create(L.getType()))};
-}
-
 void VisitTagTypeLoc(TagTypeLoc L) {
   Ref =
   ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), 
{L.getDecl()}};
@@ -495,9 +488,25 @@ Optional refInTypeLoc(Type
 }
 
 void VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc L) {
+  // We must ensure template type aliases are included in results if they
+  // were written in the source code, e.g. in
+  //template  using valias = vector;
+  //^valias x;
+  // 'explicitReferenceTargets' will return:
+  //1. valias with mask 'Alias'.
+  //2. 'vector' with mask 'Underlying'.
+  //  we want to return only #1 in this case.
   Ref = ReferenceLoc{
   NestedNameSpecifierLoc(), L.getTemplateNameLoc(),
-  explicitReferenceTargets(DynTypedNode::create(L.getType()))};
+  explicitReferenceTargets(DynTypedNode::create(L.getType()),
+   DeclRelation::Alias)};
+}
+void VisitDeducedTemplateSpecializationTypeLoc(
+DeducedTemplateSpecializationTypeLoc L) {
+  Ref = ReferenceLoc{
+  NestedNameSpecifierLoc(), L.getNameLoc(),
+  explicitReferenceTargets(DynTypedNode::create(L.getType()),
+   DeclRelation::Alias)};
 }
 
 void VisitDependentTemplateSpecializationTypeLoc(

Modified: clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp?rev=373104=373103=373104=diff
==
--- clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp Fri Sep 27 
10:55:46 2019
@@ -615,20 +615,18 @@ TEST_F(FindExplicitReferencesTest, All)
 )cpp",
"0: targets = {vector}\n"
"1: targets = {vector}\n"},
-  // FIXME: Fix 'allTargetDecls' to return alias template and 
re-enable.
   // Template type aliases.
-  //   {R"cpp(
-  //   template  struct vector { using value_type = T; };
-  //   template <> struct vector { using value_type = bool; };
-  //   template  using valias = vector;
-  //   void foo() {
-  // $0^valias vi;
-  // $1^valias vb;
-  //   }
-  // )cpp",
-  //"0: targets = {valias}\n"
-  //"1: targets = {valias}\n"},
-
+  {R"cpp(
+template  struct vector { using value_type = T; };
+template <> struct vector { using value_type = bool; };
+template  using valias = vector;
+void foo() {
+  $0^valias vi;
+  $1^valias vb;
+}
+  )cpp",
+   "0: targets = {valias}\n"
+   "1: targets = {valias}\n"},
   // MemberExpr should know their using declaration.
   {R"cpp(
 struct X { void func(int); }


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

[PATCH] D66176: Fix Driver/modules.cpp test to work when build directory name contains '.s'

[Tom Stellard via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/modules.cpp:19
+// CHECK-COMPILE-SAME: {{ -o }}
+// CHECK-COMPILE-SAME: module{{2*}}.{{pcm.o|s}}
 // CHECK-COMPILE-SAME: -x pcm

dyung wrote:
> I'm not sure what the regex module{{2*}} is supposed to match, or prevent 
> from matching? Was this intentional?
> 
> It seems it would match "module", "module2", "module22", "module222", etc. 
> When would the compiler ever generate anything other than the first? Unless 
> you are trying to protect yourself against a build directory that contains 
> module in the name, but I'm not sure how this helps that...
This CHECK line is used by two different RUN lines, the first writes to 
module.pcm.o and the second writes to module2.pcm.o.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66176



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

[PATCH] D68147: [MC][ELF] Prevent globals with an explicit section from being mergeable by default and add a safe option to allow mergeable

[ben via Phabricator via cfe-commits]

bd1976llvm created this revision.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

Alternative to https://reviews.llvm.org/D68101

Prevents globals with an explicit section from being mergeable by default (as 
in D68101 ).
Adds a command line option to allow the explicit section to be mergeable; but, 
an error will be emitted if broken output would be created.
Users can use the command line option if the default behaviour results in a 
performance regression.

This is a partial fix for https://bugs.llvm.org/show_bug.cgi?id=43457.


Repository:
  rC Clang

https://reviews.llvm.org/D68147

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/CodeGen/CommandFlags.inc
  llvm/include/llvm/Target/TargetMachine.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/Target/TargetLoweringObjectFile.cpp
  llvm/test/CodeGen/X86/explict-section-mergeable.ll
  llvm/test/CodeGen/X86/section_mergeable_size.ll

Index: llvm/test/CodeGen/X86/section_mergeable_size.ll
===
--- llvm/test/CodeGen/X86/section_mergeable_size.ll
+++ llvm/test/CodeGen/X86/section_mergeable_size.ll
@@ -1,3 +1,3 @@
-; RUN: llc -mtriple x86_64-linux-gnu < %s | FileCheck %s
+; RUN: llc -mtriple x86_64-linux-gnu -mergeable-explicit-sections < %s | FileCheck %s
 @a = internal unnamed_addr constant [1 x [1 x i32]] zeroinitializer, section ".init.rodata", align 4
 ; CHECK: .init.rodata,"aM",{{[@%]}}progbits,4
Index: llvm/test/CodeGen/X86/explict-section-mergeable.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/explict-section-mergeable.ll
@@ -0,0 +1,27 @@
+; RUN: llc < %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefix=DEFAULT
+
+;; show that the implicitly generated sections are as expected
+@implicit1 = unnamed_addr constant [2 x i16] [i16 1, i16 1]
+; DEFAULT-CHECK: .section .rodata.cst4,"aM",@progbits,4
+@implicit2 = unnamed_addr constant [2 x i32] [i32 1, i32 1]
+; DEFAULT-CHECK: .section .rodata.cst8,"aM",@progbits,8
+@implicit3 = unnamed_addr constant [2 x i16] [i16 1, i16 0]
+; DEFAULT-CHECK: .section .rodata.str2.2,"aMS",@progbits,2
+
+;; Check that an explicitly created section is not SHF_MERGE and that
+;; it contains all three symbols.
+@explicit1 = unnamed_addr constant [2 x i16] [i16 1, i16 1] "rodata-section"=".explicit"
+@explicit2 = unnamed_addr constant [2 x i32] [i32 1, i32 1] "rodata-section"=".explicit"
+@explicit3 = unnamed_addr constant [2 x i16] [i16 1, i16 0] "rodata-section"=".explicit"
+; DEFAULT: .section .explicit,"a",@progbits
+; DEFAULT-NOT: .section
+; DEFAULT: explicit1:
+; DEFAULT-NOT: .section
+; DEFAULT: explicit2:
+; DEFAULT-NOT: .section
+; DEFAULT: explicit3:
+
+; RUN: not llc < %s -mtriple=x86_64-linux-gnu -mergeable-explicit-sections 2>&1 | FileCheck %s --check-prefix=MERGE
+
+; MERGE: Symbol 'explicit2' from module '' required a section with entry-size=8 but was placed in section '.explicit' with entry-size=4: Explicit assignment by pragma or attribute of incompatible symbols to a section?
+
Index: llvm/lib/Target/TargetLoweringObjectFile.cpp
===
--- llvm/lib/Target/TargetLoweringObjectFile.cpp
+++ llvm/lib/Target/TargetLoweringObjectFile.cpp
@@ -135,6 +135,26 @@
 const MCSymbol *Sym) const {
 }
 
+static bool inExplicitSection(const GlobalObject *GO, SectionKind Kind) {
+  if (GO->hasSection())
+return true;
+
+  if (auto *GVar = dyn_cast(GO)) {
+auto Attrs = GVar->getAttributes();
+if ((Attrs.hasAttribute("bss-section") && Kind.isBSS()) ||
+(Attrs.hasAttribute("data-section") && Kind.isData()) ||
+(Attrs.hasAttribute("rodata-section") && Kind.isReadOnly())) {
+  return true;
+}
+  }
+
+  if (auto *F = dyn_cast(GO)) {
+if (F->hasFnAttribute("implicit-section-name"))
+  return true;
+  }
+
+  return false;
+}
 
 /// getKindForGlobal - This is a top-level target-independent classifier for
 /// a global object.  Given a global variable and information from the TM, this
@@ -187,6 +207,16 @@
   if (!GVar->hasGlobalUnnamedAddr())
 return SectionKind::getReadOnly();
 
+  // If two globals with differing sizes end up in the same mergeable section that
+  // section can be assigned an incorrect entry size. To avoid this we do not put
+  // globals into a mergeable section if they have been explicitly assigned to a section.
+  // TODO: This is a pessimistic solution as we lose the benifits of mergeable sections. A
+  //   better solution 

[PATCH] D66176: Fix Driver/modules.cpp test to work when build directory name contains '.s'

[Tom Stellard via Phabricator via cfe-commits]

tstellar updated this revision to Diff 05.
tstellar added a comment.

Don't check .s suffix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66176

Files:
  clang/test/Driver/modules.cpp


Index: clang/test/Driver/modules.cpp
===
--- clang/test/Driver/modules.cpp
+++ clang/test/Driver/modules.cpp
@@ -15,7 +15,7 @@
 // RUN: %clang -std=c++2a %t/module.pcm -S -o %t/module.pcm.o -v 2>&1 | 
FileCheck %s --check-prefix=CHECK-COMPILE
 //
 // CHECK-COMPILE: -cc1 {{.*}} {{-emit-obj|-S}}
-// CHECK-COMPILE-SAME: -o {{.*}}.{{pcm.o|s}}
+// CHECK-COMPILE-SAME: -o {{.*}}module{{2*}}.pcm.o
 // CHECK-COMPILE-SAME: -x pcm
 // CHECK-COMPILE-SAME: {{.*}}.pcm
 


Index: clang/test/Driver/modules.cpp
===
--- clang/test/Driver/modules.cpp
+++ clang/test/Driver/modules.cpp
@@ -15,7 +15,7 @@
 // RUN: %clang -std=c++2a %t/module.pcm -S -o %t/module.pcm.o -v 2>&1 | FileCheck %s --check-prefix=CHECK-COMPILE
 //
 // CHECK-COMPILE: -cc1 {{.*}} {{-emit-obj|-S}}
-// CHECK-COMPILE-SAME: -o {{.*}}.{{pcm.o|s}}
+// CHECK-COMPILE-SAME: -o {{.*}}module{{2*}}.pcm.o
 // CHECK-COMPILE-SAME: -x pcm
 // CHECK-COMPILE-SAME: {{.*}}.pcm
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68124: [clangd] Fix template type aliases in findExplicitReference

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Add a comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68124

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -615,20 +615,18 @@
 )cpp",
"0: targets = {vector}\n"
"1: targets = {vector}\n"},
-  // FIXME: Fix 'allTargetDecls' to return alias template and 
re-enable.
   // Template type aliases.
-  //   {R"cpp(
-  //   template  struct vector { using value_type = T; };
-  //   template <> struct vector { using value_type = bool; };
-  //   template  using valias = vector;
-  //   void foo() {
-  // $0^valias vi;
-  // $1^valias vb;
-  //   }
-  // )cpp",
-  //"0: targets = {valias}\n"
-  //"1: targets = {valias}\n"},
-
+  {R"cpp(
+template  struct vector { using value_type = T; };
+template <> struct vector { using value_type = bool; };
+template  using valias = vector;
+void foo() {
+  $0^valias vi;
+  $1^valias vb;
+}
+  )cpp",
+   "0: targets = {valias}\n"
+   "1: targets = {valias}\n"},
   // MemberExpr should know their using declaration.
   {R"cpp(
 struct X { void func(int); }
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -477,13 +477,6 @@
   Ref->Qualifier = L.getQualifierLoc();
 }
 
-void VisitDeducedTemplateSpecializationTypeLoc(
-DeducedTemplateSpecializationTypeLoc L) {
-  Ref = ReferenceLoc{
-  NestedNameSpecifierLoc(), L.getNameLoc(),
-  explicitReferenceTargets(DynTypedNode::create(L.getType()))};
-}
-
 void VisitTagTypeLoc(TagTypeLoc L) {
   Ref =
   ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), 
{L.getDecl()}};
@@ -495,9 +488,25 @@
 }
 
 void VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc L) {
+  // We must ensure template type aliases are included in results if they
+  // were written in the source code, e.g. in
+  //template  using valias = vector;
+  //^valias x;
+  // 'explicitReferenceTargets' will return:
+  //1. valias with mask 'Alias'.
+  //2. 'vector' with mask 'Underlying'.
+  //  we want to return only #1 in this case.
   Ref = ReferenceLoc{
   NestedNameSpecifierLoc(), L.getTemplateNameLoc(),
-  explicitReferenceTargets(DynTypedNode::create(L.getType()))};
+  explicitReferenceTargets(DynTypedNode::create(L.getType()),
+   DeclRelation::Alias)};
+}
+void VisitDeducedTemplateSpecializationTypeLoc(
+DeducedTemplateSpecializationTypeLoc L) {
+  Ref = ReferenceLoc{
+  NestedNameSpecifierLoc(), L.getNameLoc(),
+  explicitReferenceTargets(DynTypedNode::create(L.getType()),
+   DeclRelation::Alias)};
 }
 
 void VisitDependentTemplateSpecializationTypeLoc(


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -615,20 +615,18 @@
 )cpp",
"0: targets = {vector}\n"
"1: targets = {vector}\n"},
-  // FIXME: Fix 'allTargetDecls' to return alias template and re-enable.
   // Template type aliases.
-  //   {R"cpp(
-  //   template  struct vector { using value_type = T; };
-  //   template <> struct vector { using value_type = bool; };
-  //   template  using valias = vector;
-  //   void foo() {
-  // $0^valias vi;
-  // $1^valias vb;
-  //   }
-  // )cpp",
-  //"0: targets = {valias}\n"
-  //"1: targets = {valias}\n"},
-
+  {R"cpp(
+template  struct vector { using value_type = T; };
+template <> struct vector { using value_type = bool; };
+template  using valias = vector;
+void foo() {
+  $0^valias vi;
+  $1^valias vb;
+}
+  )cpp",
+   "0: targets = {valias}\n"
+   "1: targets = {valias}\n"},
   // MemberExpr should know their 

[PATCH] D64319: [OpenCL] Add function attributes handling for builtin functions

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh updated this revision to Diff 222197.
svenvh edited the summary of this revision.
svenvh added a comment.
Herald added a reviewer: rengolin.

- Rebase onto recent master.
- Fix formatting.
- Use predefined attribute sets like "Attr.Const" instead of bit lists.


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

https://reviews.llvm.org/D64319

Files:
  clang/lib/Sema/OpenCLBuiltins.td
  clang/lib/Sema/SemaLookup.cpp
  clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
  clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Index: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
===
--- clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
+++ clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -270,6 +270,12 @@
   // the SignatureTable represent the complete signature.  The first type at
   // index SigTableIndex is the return type.
   const unsigned NumTypes;
+  // Function attribute __attribute__((pure))
+  const bool IsPure;
+  // Function attribute __attribute__((const))
+  const bool IsConst;
+  // Function attribute __attribute__((convergent))
+  const bool IsConv;
   // First OpenCL version in which this overload was introduced (e.g. CL20).
   const unsigned short MinVersion;
   // First OpenCL version in which this overload was removed (e.g. CL20).
@@ -408,6 +414,9 @@
 for (const auto  : FOM.second) {
   OS << "  { " << Overload.second << ", "
  << Overload.first->getValueAsListOfDefs("Signature").size() << ", "
+ << (Overload.first->getValueAsBit("IsPure")) << ", "
+ << (Overload.first->getValueAsBit("IsConst")) << ", "
+ << (Overload.first->getValueAsBit("IsConv")) << ", "
  << Overload.first->getValueAsDef("MinVersion")->getValueAsInt("ID")
  << ", "
  << Overload.first->getValueAsDef("MaxVersion")->getValueAsInt("ID")
Index: clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -emit-llvm -o - -O0 -triple spir-unknown-unknown -fdeclare-opencl-builtins -finclude-default-header %s | FileCheck %s
+
+// Test that Attr.Const from OpenCLBuiltins.td is lowered to a readnone attribute.
+// CHECK-LABEL: @test_const_attr
+// CHECK: call i32 @_Z3maxii({{.*}}) [[ATTR_CONST:#[0-9]]]
+// CHECK: ret
+int test_const_attr(int a) {
+  return max(a, 2);
+}
+
+// Test that Attr.Pure from OpenCLBuiltins.td is lowered to a readonly attribute.
+// CHECK-LABEL: @test_pure_attr
+// CHECK: call <4 x float> @_Z11read_imagef{{.*}} [[ATTR_PURE:#[0-9]]]
+// CHECK: ret
+kernel void test_pure_attr(read_only image1d_t img) {
+  float4 resf = read_imagef(img, 42);
+}
+
+// CHECK: attributes [[ATTR_CONST]] =
+// CHECK-SAME: readnone
+// CHECK: attributes [[ATTR_PURE]] =
+// CHECK-SAME: readonly
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -812,9 +812,17 @@
 }
 NewOpenCLBuiltin->setParams(ParmList);
   }
-  if (!S.getLangOpts().OpenCLCPlusPlus) {
+
+  // Add function attributes.
+  if (OpenCLBuiltin.IsPure)
+NewOpenCLBuiltin->addAttr(PureAttr::CreateImplicit(Context));
+  if (OpenCLBuiltin.IsConst)
+NewOpenCLBuiltin->addAttr(ConstAttr::CreateImplicit(Context));
+  if (OpenCLBuiltin.IsConv)
+NewOpenCLBuiltin->addAttr(ConvergentAttr::CreateImplicit(Context));
+  if ((GenTypeMaxCnt > 1 || Len > 1) && !S.getLangOpts().OpenCLCPlusPlus)
 NewOpenCLBuiltin->addAttr(OverloadableAttr::CreateImplicit(Context));
-  }
+
   LR.addDecl(NewOpenCLBuiltin);
 }
   }
Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -180,10 +180,18 @@
   let VecWidth = 0;
 }
 
+// Builtin function attributes.
+def Attr {
+  list None = [0, 0, 0];
+  list Pure = [1, 0, 0];
+  list Const = [0, 1, 0];
+  list Convergent = [0, 0, 1];
+}
+
 //===--===//
 //  OpenCL C class for builtin functions
 //===--===//
-class Builtin _Signature> {
+class Builtin _Signature, list _Attributes = Attr.None> {
   // Name of the builtin function
   string Name = _Name;
   // List of types used by the function. The first one is the return type and
@@ -192,6 +200,12 @@
   list Signature = _Signature;
   // OpenCL Extension to which the function belongs (cl_khr_subgroups, ...)
   string Extension = "";
+  // Function attribute __attribute__((pure))
+  bit IsPure = _Attributes[0];
+  // Function attribute __attribute__((const))
+  bit IsConst = _Attributes[1];
+  // 

[PATCH] D68028: [clang] Add no_builtin attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked an inline comment as done.
aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1092
+  // Wildcard is a super set of all builtins, we keep only this one.
+  if (FunctionNames.count(Wildcard) > 0) {
+FunctionNames.clear();

gchatelet wrote:
> aaron.ballman wrote:
> > This silently changes the behavior from what the user might expect, which 
> > seems bad. For instance, it would be very reasonable for a user to write 
> > `[[clang::no_builtin("__builtin_mem*")]]` expecting that to behave as a 
> > regex pattern, but instead it silently turns into a "disable all builtins".
> > 
> > I think it makes sense to diagnose unexpected input like that rather than 
> > silently accept it under perhaps unexpected semantics. It may also make 
> > sense to support regex patterns in the strings or at least keep the design 
> > such that we can add that functionality later without breaking users.
> The code looks for a function name that is exactly `*` and not "a function 
> name that contains `*`", it would not turn 
> `[[clang::no_builtin("__builtin_mem*")]]` into `[[clang::no_builtin("*")]]`.
> That said, I fully agree that an error should be returned if the function 
> name is not valid.
> I'll work on this.
Ah, I missed that -- great!



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1072
+NoBuiltinAttr *
+Sema::mergeNoBuiltinAttr(Sema , Decl *D, const AttributeCommonInfo ,
+ llvm::ArrayRef FunctionNames) {

You're missing a call to this function within `mergeDeclAttribute()` in 
SemaDecl.cpp.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1078-1079
+  // Insert previous NoBuiltin attributes.
+  if (D->hasAttr())
+for (StringRef FunctionName : D->getAttr()->functionNames())
+  FunctionNamesSet.insert(FunctionName);

Instead of doing `hasAttr<>` followed by `getAttr<>`, this should be:
```
if (const auto *NBA = D->getAttr()) {
  for (StringRef FunctionName : NBA->functionNames())
...
}
```
But are you able to use `llvm::copy()` instead of a manual loop?



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1082-1083
+  // Insert new NoBuiltin attributes.
+  for (StringRef FunctionName : FunctionNames)
+FunctionNamesSet.insert(FunctionName);
+

Same question here about using `llvm::copy()`.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1086-1089
+  if (FunctionNamesSet.count(Wildcard) > 0) {
+FunctionNamesSet.clear();
+FunctionNamesSet.insert(Wildcard);
+  }

Rather than walking the entire set like this, would it make more sense to look 
for the wildcard in the above loop before inserting the name, and set a local 
variable if found, so that you can do the clear without having to rescan the 
entire list?



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1098-1099
+
+  if (D->hasAttr())
+D->dropAttr();
+

Just making sure I understand the semantics you want: redeclarations do not 
have to have matching lists of builtins, but instead the definition will use a 
merged list? e.g.,
```
[[clang::no_builtin("memset")]] void whatever();
[[clang::no_builtin("memcpy")]] void whatever();

[[clang::no_builtin("memmove")]] void whatever() {
 // Will not use memset, memcpy, or memmove builtins.
}
```
That seems a bit strange, to me. In fact, being able to apply this attribute to 
a declaration seems a bit like a mistake -- this only impacts the definition of 
the function, and I can't imagine good things coming from hypothetical code 
like:
```
[[clang::no_builtin("memset")]] void whatever();
#include "whatever.h" // Provides a library declaration of whatever() with no 
restrictions on it
```
WDYT about restricting this attribute to only appear on definitions?



Comment at: clang/test/Sema/no-builtin.c:1
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - -verify %s
+

This should not be emitting LLVM -- it should be `-fsyntax-only`, however.



Comment at: clang/test/Sema/no-builtin.c:9
+
+int __attribute__((no_builtin("*"))) variable; // expected-warning 
{{'no_builtin' attribute only applies to functions}}

We'll need more tests for redeclarations or declaration vs definition once we 
decide how we want to handle that.

You should also add some positive tests that show the attribute applying as 
expected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68028



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

[PATCH] D68024: [clangd] Implement GetEligiblePoints

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/SourceCode.h:270
+  /// It will be “a::b” for both carrot locations.
+  std::string CurrentNamespace;
+  /// Offsets into the code marking eligible points to insert a function

hokein wrote:
> `Current` seems not clear enough, how about "MatchNamespace"?
what about enclosing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68024



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

[PATCH] D67436: CodeGen: set correct result for atomic compound expressions

[JF Bastien via Phabricator via cfe-commits]

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

Separately, does this do floating-point add / sub properly? We added them too 
C++20.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67436



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

[PATCH] D68124: [clangd] Fix template type aliases in findExplicitReference

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

LGTM.




Comment at: clang-tools-extra/clangd/FindTarget.cpp:478
+  explicitReferenceTargets(DynTypedNode::create(L.getType()),
+   DeclRelation::Alias)};
 }

ilya-biryukov wrote:
> kadircet wrote:
> > can you explain why these two are special and get to keep `Alias` decls? 
> > Whereas the others dont?
> There are two cases to consider for a reference of the form `Templ` in 
> type positions:
> 1. `Templ` is a template type alias
> ```
> template 
> Templ = vector;
> ```
> in this case `targetDecl` returns `Templ` with mask `Alias | TemplatePattern` 
>  and `vector` with mask `Underlying`
> since we are interested in the thing explicitly written in the source code, 
> we want `Templ` and **not** `vector`.
> 
> 2. `Templ` is a name of some template type (a class or template template 
> parameter)
> in this case `targetDecl` returns `Templ` with mask `TemplatePattern` and 
> that's what we want to return, since this is what was written in the source 
> code.
> 
> So in both cases the important thing is to **not** filter-out the result with 
> mask `Alias`. This is exactly what this patch does.
thanks! I was rather asking for comments in the code though :D

Please also mention them in comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68124



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

[PATCH] D67907: [clangd] Implement a smart version of HeaderSource switch.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks for formatting test cases, I had it in mind but forgot to mention in 
last round.

LGTM




Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:113
+  auto Best = Candidates.begin();
+  for (auto It = Candidates.begin(); It != Candidates.end(); ++It) {
+if (It->second > Best->second)

nit: `for(auto  : Candidates)`



Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:117
+// Select the first one in the lexical order if we have multiple 
candidates.
+if (It->second == Best->second && It->first() < Best->first())
+  Best = It;

nit: `else if`



Comment at: clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp:196
+  } TestCases[] = {
+  {R"cpp(// empty, no header found)cpp", llvm::None},
+  {R"cpp(

kadircet wrote:
> again no need for raw literals
not done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67907



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

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.

Clang uses the location identifier should be inserted for declarator
decls when a decl is unnamed. But for type template and template template
paramaters it uses the location of "typename/class" keyword, which makes it hard
for tooling to insert/change parameter names.

This change tries to unify these two cases by making template parameter
parsing and sourcerange operations similar to function params/declarator decls.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68143

Files:
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-record-definition-data-json.cpp
  clang/test/AST/ast-dump-template-decls-json.cpp
  clang/test/AST/ast-dump-template-decls.cpp
  clang/test/ASTMerge/class-template/test.cpp
  clang/test/Index/index-templates.cpp

Index: clang/test/Index/index-templates.cpp
===
--- clang/test/Index/index-templates.cpp
+++ clang/test/Index/index-templates.cpp
@@ -155,7 +155,7 @@
 // CHECK-LOAD: index-templates.cpp:36:44: DeclRefExpr=OneDimension:35:16 Extent=[36:44 - 36:56]
 // CHECK-LOAD: index-templates.cpp:40:8: ClassTemplate=storage:40:8 (Definition) Extent=[39:1 - 40:19]
 // CHECK-LOAD: index-templates.cpp:39:45: TemplateTemplateParameter=DataStructure:39:45 (Definition) Extent=[39:10 - 39:66]
-// CHECK-LOAD: index-templates.cpp:39:19: TemplateTypeParameter=:39:19 (Definition) Extent=[39:19 - 39:27]
+// CHECK-LOAD: index-templates.cpp:39:27: TemplateTypeParameter=:39:27 (Definition) Extent=[39:19 - 39:27]
 // CHECK-LOAD: index-templates.cpp:39:37: NonTypeTemplateParameter=:39:37 (Definition) Extent=[39:29 - 39:37]
 // CHECK-LOAD: index-templates.cpp:39:61: TemplateRef=array:37:8 Extent=[39:61 - 39:66]
 // CHECK-LOAD: index-templates.cpp:42:18: TypedefDecl=Unsigned:42:18 (Definition) Extent=[42:1 - 42:26]
Index: clang/test/ASTMerge/class-template/test.cpp
===
--- clang/test/ASTMerge/class-template/test.cpp
+++ clang/test/ASTMerge/class-template/test.cpp
@@ -9,13 +9,13 @@
 // CHECK: class-template2.cpp:9:15: note: declared here with type 'long'
 
 // CHECK: class-template1.cpp:12:14: warning: template parameter has different kinds in different translation units
-// CHECK: class-template2.cpp:12:10: note: template parameter declared here
+// CHECK: class-template2.cpp:12:18: note: template parameter declared here
 
 // CHECK: class-template1.cpp:18:23: warning: non-type template parameter declared with incompatible types in different translation units ('long' vs. 'int')
 // CHECK: class-template2.cpp:18:23: note: declared here with type 'int'
 
-// CHECK: class-template1.cpp:21:10: warning: template parameter has different kinds in different translation units
-// CHECK: class-template2.cpp:21:10: note: template parameter declared here
+// CHECK: class-template1.cpp:21:18: warning: template parameter has different kinds in different translation units
+// CHECK: class-template2.cpp:21:31: note: template parameter declared here
 
 // CHECK: class-template2.cpp:27:20: warning: external variable 'x0r' declared with incompatible types in different translation units ('X0 *' vs. 'X0 *')
 // CHECK: class-template1.cpp:26:19: note: declared here with type 'X0 *'
Index: clang/test/AST/ast-dump-template-decls.cpp
===
--- clang/test/AST/ast-dump-template-decls.cpp
+++ clang/test/AST/ast-dump-template-decls.cpp
@@ -26,7 +26,7 @@
 // CHECK: FunctionTemplateDecl 0x{{[^ ]*}}  col:6 d
 // CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:20 referenced typename depth 0 index 0 Ty
 // CHECK-NEXT: TemplateTemplateParmDecl 0x{{[^ ]*}}  col:52 depth 0 index 1 Uy
-// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:33 typename depth 1 index 0
+// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:41 typename depth 1 index 0
 void d(Ty, Uy);
 
 template 
@@ -47,7 +47,7 @@
 
 template 
 // CHECK: FunctionTemplateDecl 0x{{[^ ]*}}  col:6 h
-// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:11 typename depth 0 index 0
+// CHECK-NEXT: TemplateTypeParmDecl 0x{{[^ ]*}}  col:20 typename depth 0 index 0
 // CHECK-NEXT: TemplateArgument type 'void'
 void h();
 
Index: clang/test/AST/ast-dump-template-decls-json.cpp
===
--- clang/test/AST/ast-dump-template-decls-json.cpp
+++ clang/test/AST/ast-dump-template-decls-json.cpp
@@ -656,8 +656,8 @@
 // CHECK-NEXT:"id": "0x{{.*}}",
 // CHECK-NEXT:"kind": "TemplateTypeParmDecl",
 // CHECK-NEXT:"loc": {
-// CHECK-NEXT: "col": 33,
-// CHECK-NEXT: "tokLen": 8
+// CHECK-NEXT: "col": 41,
+// CHECK-NEXT: 

[PATCH] D68142: [Alignment][NFC] Remove LoadInst::setAlignment(unsigned)

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet created this revision.
gchatelet added a reviewer: courbet.
Herald added subscribers: llvm-commits, cfe-commits, asbirlea, hiraditya.
Herald added a reviewer: jdoerfert.
Herald added projects: clang, LLVM.

This is patch is part of a series to introduce an Alignment type.
See this thread for context: 
http://lists.llvm.org/pipermail/llvm-dev/2019-July/133851.html
See this patch for the introduction of the type: https://reviews.llvm.org/D64790


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68142

Files:
  clang/lib/CodeGen/CGCleanup.cpp
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/Instructions.h
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
  llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
  llvm/lib/Transforms/Scalar/GVNHoist.cpp
  llvm/lib/Transforms/Scalar/LICM.cpp
  llvm/lib/Transforms/Scalar/SROA.cpp
  llvm/lib/Transforms/Utils/VNCoercion.cpp
  llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

Index: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
===
--- llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -3951,11 +3951,10 @@
   if (getTreeEntry(PO))
 ExternalUses.push_back(ExternalUser(PO, cast(VecPtr), 0));
 
-  unsigned Alignment = LI->getAlignment();
+  MaybeAlign Alignment = MaybeAlign(LI->getAlignment());
   LI = Builder.CreateLoad(VecTy, VecPtr);
-  if (!Alignment) {
-Alignment = DL->getABITypeAlignment(ScalarLoadTy);
-  }
+  if (!Alignment)
+Alignment = MaybeAlign(DL->getABITypeAlignment(ScalarLoadTy));
   LI->setAlignment(Alignment);
   Value *V = propagateMetadata(LI, E->Scalars);
   if (IsReorder) {
Index: llvm/lib/Transforms/Utils/VNCoercion.cpp
===
--- llvm/lib/Transforms/Utils/VNCoercion.cpp
+++ llvm/lib/Transforms/Utils/VNCoercion.cpp
@@ -431,7 +431,7 @@
 PtrVal = Builder.CreateBitCast(PtrVal, DestPTy);
 LoadInst *NewLoad = Builder.CreateLoad(DestTy, PtrVal);
 NewLoad->takeName(SrcVal);
-NewLoad->setAlignment(SrcVal->getAlignment());
+NewLoad->setAlignment(MaybeAlign(SrcVal->getAlignment()));
 
 LLVM_DEBUG(dbgs() << "GVN WIDENED LOAD: " << *SrcVal << "\n");
 LLVM_DEBUG(dbgs() << "TO: " << *NewLoad << "\n");
Index: llvm/lib/Transforms/Scalar/SROA.cpp
===
--- llvm/lib/Transforms/Scalar/SROA.cpp
+++ llvm/lib/Transforms/Scalar/SROA.cpp
@@ -1270,7 +1270,7 @@
   // matter which one we get and if any differ.
   AAMDNodes AATags;
   SomeLoad->getAAMetadata(AATags);
-  unsigned Align = SomeLoad->getAlignment();
+  MaybeAlign Align = MaybeAlign(SomeLoad->getAlignment());
 
   // Rewrite all loads of the PN to use the new PHI.
   while (!PN.use_empty()) {
@@ -1368,8 +1368,8 @@
 NumLoadsSpeculated += 2;
 
 // Transfer alignment and AA info if present.
-TL->setAlignment(LI->getAlignment());
-FL->setAlignment(LI->getAlignment());
+TL->setAlignment(MaybeAlign(LI->getAlignment()));
+FL->setAlignment(MaybeAlign(LI->getAlignment()));
 
 AAMDNodes Tags;
 LI->getAAMetadata(Tags);
@@ -3118,7 +3118,7 @@
 unsigned LoadAlign = LI->getAlignment();
 if (!LoadAlign)
   LoadAlign = DL.getABITypeAlignment(LI->getType());
-LI->setAlignment(std::min(LoadAlign, getSliceAlign()));
+LI->setAlignment(MaybeAlign(std::min(LoadAlign, getSliceAlign(;
 continue;
   }
   if (StoreInst *SI = dyn_cast(I)) {
Index: llvm/lib/Transforms/Scalar/LICM.cpp
===
--- llvm/lib/Transforms/Scalar/LICM.cpp
+++ llvm/lib/Transforms/Scalar/LICM.cpp
@@ -2109,7 +2109,7 @@
   SomePtr->getName() + ".promoted", Preheader->getTerminator());
   if (SawUnorderedAtomic)
 PreheaderLoad->setOrdering(AtomicOrdering::Unordered);
-  PreheaderLoad->setAlignment(Alignment);
+  PreheaderLoad->setAlignment(MaybeAlign(Alignment));
   PreheaderLoad->setDebugLoc(DL);
   if (AATags)
 PreheaderLoad->setAAMetadata(AATags);
Index: llvm/lib/Transforms/Scalar/GVNHoist.cpp
===
--- llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -889,9 +889,8 @@
 
   void updateAlignment(Instruction *I, Instruction *Repl) {
 if (auto *ReplacementLoad = dyn_cast(Repl)) {
-  ReplacementLoad->setAlignment(
-  std::min(ReplacementLoad->getAlignment(),
-  

[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Add renaming of template and function parameters


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66647

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.h
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -25,6 +25,7 @@
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -103,7 +104,7 @@
   EXPECT_UNAVAILABLE(R"cpp(R"(multi )" ^"token " u8"str\ning")cpp"); // nonascii
   EXPECT_UNAVAILABLE(R"cpp(^R^"^(^multi )" "token " "str\ning")cpp"); // raw
   EXPECT_UNAVAILABLE(R"cpp(^"token\n" __FILE__)cpp"); // chunk is macro
-  EXPECT_UNAVAILABLE(R"cpp(^"a\r\n";)cpp"); // forbidden escape char
+  EXPECT_UNAVAILABLE(R"cpp(^"a\r\n";)cpp");   // forbidden escape char
 
   const char *Input = R"cpp(R"(multi
 token)" "\nst^ring\n" "literal")cpp";
@@ -815,6 +816,497 @@
   })cpp");
 }
 
+TEST_F(DefineInlineTest, TransformUsings) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a {
+  void bar();
+  namespace b {
+  void baz();
+  namespace c {
+  void aux();
+  }
+  }
+  }
+
+  void foo();
+  void f^oo() {
+using namespace a;
+
+using namespace b;
+using namespace a::b;
+
+using namespace c;
+using namespace b::c;
+using namespace a::b::c;
+
+using a::bar;
+
+using b::baz;
+using a::b::baz;
+
+using c::aux;
+using b::c::aux;
+using a::b::c::aux;
+
+namespace d = c;
+namespace d = b::c;
+  }
+  )cpp"),
+R"cpp(
+  namespace a {
+  void bar();
+  namespace b {
+  void baz();
+  namespace c {
+  void aux();
+  }
+  }
+  }
+
+  void foo(){
+using namespace a;
+
+using namespace a::b;
+using namespace a::b;
+
+using namespace a::b::c;
+using namespace a::b::c;
+using namespace a::b::c;
+
+using a::bar;
+
+using a::b::baz;
+using a::b::baz;
+
+using a::b::c::aux;
+using a::b::c::aux;
+using a::b::c::aux;
+
+namespace d = a::b::c;
+namespace d = a::b::c;
+  }
+  
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformDecls) {
+  EXPECT_EQ(apply(R"cpp(
+  void foo()/*Comment -_-*/  ;
+
+  void f^oo() {
+class Foo {
+public:
+  void foo();
+  int x;
+  static int y;
+};
+Foo::y = 0;
+
+enum En { Zero, One };
+En x = Zero;
+
+enum class EnClass { Zero, One };
+EnClass y = EnClass::Zero;
+
+template  class Bar {};
+Bar z;
+  }
+  )cpp"),
+R"cpp(
+  void foo()/*Comment -_-*/  {
+class Foo {
+public:
+  void foo();
+  int x;
+  static int y;
+};
+Foo::y = 0;
+
+enum En { Zero, One };
+En x = Zero;
+
+enum class EnClass { Zero, One };
+EnClass y = EnClass::Zero;
+
+template  class Bar {};
+Bar z;
+  }
+
+  
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, TransformTemplDecls) {
+  EXPECT_EQ(apply(R"cpp(
+  namespace a {
+template  class Bar {
+public:
+  void bar();
+};
+template  T bar;
+template  void aux() {}
+  }
+
+  void foo()/*Comment -_-*/  ;
+
+  void f^oo() {
+using namespace a;
+bar>.bar();
+aux>();
+  }
+  )cpp"),
+R"cpp(
+  namespace a {
+template  class Bar {
+public:
+  void bar();
+};
+template  T bar;
+template  void aux() {}
+  }
+
+  void foo()/*Comment -_-*/  {
+using namespace a;
+a::bar>.bar();
+a::aux>();
+  }
+
+  
+  )cpp");
+}
+
+MATCHER_P2(FileWithContents, FileName, Contents, "") {
+  return arg.first() == FileName && arg.second == Contents;
+}
+
+TEST_F(DefineInlineTest, TransformMembers) {
+  EXPECT_EQ(apply(R"cpp(
+  class Foo {
+void foo()/*Comment -_-*/  ;
+  };
+
+  void Foo::f^oo() {
+return;
+  }
+  )cpp"),
+R"cpp(
+  class Foo {
+void foo()/*Comment -_-*/  {
+return;
+  }
+  };
+
+  
+  )cpp");
+
+  ExtraFiles["a.h"] = R"cpp(
+  class Foo {
+void foo()/*Comment -_-*/  ;
+  };)cpp";
+  EXPECT_EQ(apply(R"cpp(
+  #include "a.h"
+  void Foo::f^oo() {
+return;
+  })cpp"),
+R"cpp(
+  #include "a.h"
+  )cpp");
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(testPath("a.h"),
+R"cpp(
+  class Foo {
+void foo()/*Comment -_-*/  {
+return;
+  }
+  };)cpp")));
+}
+
+TEST_F(DefineInlineTest, TransformDependentTypes) 

[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D66647#1684046 , @ilya-biryukov 
wrote:

> We also need to rename parameters sometimes, right?
>
>   // Sometimes we need to rename parameters.
>   void usages(int decl_param, int);
>  
>   void usages(int def_param, int now_named) {
> llvm::errs() << def_param + now_named;
>   }
>  
>   // And template parameters! (these are even more interesting)
>   template 
>   struct Foo {
> template 
> void usages();
>   };
>   template 
>   template 
>   void Foo::usages() {
> llvm::errs() << L() + R() + NowNamed();
>   }
>


So currently AST doesn't store any information regarding template parameter 
locations except the deepest one.
Therefore I've changed the availability to discard any methods inside templated 
classes, since there is no way to
validate the template parameter names. Hopefully this should be a rare 
use-case, and I believe most of the times
people have same template paramater names on declaration and definition. Let me 
know what you think about
it.

In addition to that implemented renaming for function parameters and template 
parameters in case of templated
functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66647



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

[PATCH] D66652: [libTooling] Transformer: refine `SourceLocation` specified as anchor of changes.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373093: [libTooling] Transformer: refine `SourceLocation` 
specified as anchor of… (authored by ymandel, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66652?vs=222177=222179#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66652

Files:
  cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
  cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
  cfe/trunk/unittests/Tooling/TransformerTest.cpp

Index: cfe/trunk/unittests/Tooling/TransformerTest.cpp
===
--- cfe/trunk/unittests/Tooling/TransformerTest.cpp
+++ cfe/trunk/unittests/Tooling/TransformerTest.cpp
@@ -710,6 +710,57 @@
   testRule(ruleStrlenSize(), Input, Expected);
 }
 
+// Tests that two changes in a single macro expansion do not lead to conflicts
+// in applying the changes.
+TEST_F(TransformerTest, TwoChangesInOneMacroExpansion) {
+  std::string Input = R"cc(
+#define PLUS(a,b) (a) + (b)
+int f() { return PLUS(3, 4); }
+  )cc";
+  std::string Expected = R"cc(
+#define PLUS(a,b) (a) + (b)
+int f() { return PLUS(LIT, LIT); }
+  )cc";
+
+  testRule(makeRule(integerLiteral(), change(text("LIT"))), Input, Expected);
+}
+
+// Tests case where the rule's match spans both source from the macro and its
+// arg, with the begin location (the "anchor") being the arg.
+TEST_F(TransformerTest, MatchSpansMacroTextButChangeDoesNot) {
+  std::string Input = R"cc(
+#define PLUS_ONE(a) a + 1
+int f() { return PLUS_ONE(3); }
+  )cc";
+  std::string Expected = R"cc(
+#define PLUS_ONE(a) a + 1
+int f() { return PLUS_ONE(LIT); }
+  )cc";
+
+  StringRef E = "expr";
+  testRule(makeRule(binaryOperator(hasLHS(expr().bind(E))),
+change(node(E), text("LIT"))),
+   Input, Expected);
+}
+
+// Tests case where the rule's match spans both source from the macro and its
+// arg, with the begin location (the "anchor") being inside the macro.
+TEST_F(TransformerTest, MatchSpansMacroTextButChangeDoesNotAnchoredInMacro) {
+  std::string Input = R"cc(
+#define PLUS_ONE(a) 1 + a
+int f() { return PLUS_ONE(3); }
+  )cc";
+  std::string Expected = R"cc(
+#define PLUS_ONE(a) 1 + a
+int f() { return PLUS_ONE(LIT); }
+  )cc";
+
+  StringRef E = "expr";
+  testRule(makeRule(binaryOperator(hasRHS(expr().bind(E))),
+change(node(E), text("LIT"))),
+   Input, Expected);
+}
+
 // No rewrite is applied when the changed text does not encompass the entirety
 // of the expanded text. That is, the edit would have to be applied to the
 // macro's definition to succeed and editing the expansion point would not
Index: cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
===
--- cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
+++ cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
@@ -251,6 +251,12 @@
 std::vector
 buildMatchers(const RewriteRule );
 
+/// Gets the beginning location of the source matched by a rewrite rule. If the
+/// match occurs within a macro expansion, returns the beginning of the
+/// expansion point. `Result` must come from the matching of a rewrite rule.
+SourceLocation
+getRuleMatchLoc(const ast_matchers::MatchFinder::MatchResult );
+
 /// Returns the \c Case of \c Rule that was selected in the match result.
 /// Assumes a matcher built with \c buildMatcher.
 const RewriteRule::Case &
Index: cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
@@ -150,6 +150,21 @@
   return Ms[0];
 }
 
+SourceLocation tooling::detail::getRuleMatchLoc(const MatchResult ) {
+  auto  = Result.Nodes.getMap();
+  auto Root = NodesMap.find(RewriteRule::RootID);
+  assert(Root != NodesMap.end() && "Transformation failed: missing root node.");
+  llvm::Optional RootRange = getRangeForEdit(
+  CharSourceRange::getTokenRange(Root->second.getSourceRange()),
+  *Result.Context);
+  if (RootRange)
+return RootRange->getBegin();
+  // The match doesn't have a coherent range, so fall back to the expansion
+  // location as the "beginning" of the match.
+  return Result.SourceManager->getExpansionLoc(
+  Root->second.getSourceRange().getBegin());
+}
+
 // Finds the case that was "selected" -- that is, whose matcher triggered the
 // `MatchResult`.
 const RewriteRule::Case &
@@ -178,14 +193,6 @@
   if (Result.Context->getDiagnostics().hasErrorOccurred())
 return;
 
-  // Verify the existence and validity of the AST node that roots this rule.
-  auto  = Result.Nodes.getMap();
-  auto Root = NodesMap.find(RewriteRule::RootID);
-  assert(Root != 

r373093 - [libTooling] Transformer: refine `SourceLocation` specified as anchor of changes.

[Yitzhak Mandelbaum via cfe-commits]

Author: ymandel
Date: Fri Sep 27 08:26:04 2019
New Revision: 373093

URL: http://llvm.org/viewvc/llvm-project?rev=373093=rev
Log:
[libTooling] Transformer: refine `SourceLocation` specified as anchor of 
changes.

Summary: Every change triggered by a rewrite rule is anchored at a particular
location in the source code.  This patch refines how that location is chosen and
defines it as an explicit function so it can be shared by other Transformer
implementations.

This patch was inspired by a bug found by a clang tidy, wherein two changes were
anchored at the same location (the expansion loc of the macro) resulting in the
discarding of the second change.

Reviewers: gribozavr

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
cfe/trunk/unittests/Tooling/TransformerTest.cpp

Modified: cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h?rev=373093=373092=373093=diff
==
--- cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/Transformer.h Fri Sep 27 
08:26:04 2019
@@ -251,6 +251,12 @@ ast_matchers::internal::DynTypedMatcher
 std::vector
 buildMatchers(const RewriteRule );
 
+/// Gets the beginning location of the source matched by a rewrite rule. If the
+/// match occurs within a macro expansion, returns the beginning of the
+/// expansion point. `Result` must come from the matching of a rewrite rule.
+SourceLocation
+getRuleMatchLoc(const ast_matchers::MatchFinder::MatchResult );
+
 /// Returns the \c Case of \c Rule that was selected in the match result.
 /// Assumes a matcher built with \c buildMatcher.
 const RewriteRule::Case &

Modified: cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp?rev=373093=373092=373093=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp Fri Sep 27 08:26:04 2019
@@ -150,6 +150,21 @@ DynTypedMatcher tooling::detail::buildMa
   return Ms[0];
 }
 
+SourceLocation tooling::detail::getRuleMatchLoc(const MatchResult ) {
+  auto  = Result.Nodes.getMap();
+  auto Root = NodesMap.find(RewriteRule::RootID);
+  assert(Root != NodesMap.end() && "Transformation failed: missing root 
node.");
+  llvm::Optional RootRange = getRangeForEdit(
+  CharSourceRange::getTokenRange(Root->second.getSourceRange()),
+  *Result.Context);
+  if (RootRange)
+return RootRange->getBegin();
+  // The match doesn't have a coherent range, so fall back to the expansion
+  // location as the "beginning" of the match.
+  return Result.SourceManager->getExpansionLoc(
+  Root->second.getSourceRange().getBegin());
+}
+
 // Finds the case that was "selected" -- that is, whose matcher triggered the
 // `MatchResult`.
 const RewriteRule::Case &
@@ -178,14 +193,6 @@ void Transformer::run(const MatchResult
   if (Result.Context->getDiagnostics().hasErrorOccurred())
 return;
 
-  // Verify the existence and validity of the AST node that roots this rule.
-  auto  = Result.Nodes.getMap();
-  auto Root = NodesMap.find(RewriteRule::RootID);
-  assert(Root != NodesMap.end() && "Transformation failed: missing root 
node.");
-  SourceLocation RootLoc = Result.SourceManager->getExpansionLoc(
-  Root->second.getSourceRange().getBegin());
-  assert(RootLoc.isValid() && "Invalid location for Root node of match.");
-
   RewriteRule::Case Case = tooling::detail::findSelectedCase(Result, Rule);
   auto Transformations = tooling::detail::translateEdits(Result, Case.Edits);
   if (!Transformations) {
@@ -195,14 +202,16 @@ void Transformer::run(const MatchResult
 
   if (Transformations->empty()) {
 // No rewrite applied (but no error encountered either).
-RootLoc.print(llvm::errs() << "note: skipping match at loc ",
-  *Result.SourceManager);
+detail::getRuleMatchLoc(Result).print(
+llvm::errs() << "note: skipping match at loc ", *Result.SourceManager);
 llvm::errs() << "\n";
 return;
   }
 
-  // Record the results in the AtomicChange.
-  AtomicChange AC(*Result.SourceManager, RootLoc);
+  // Record the results in the AtomicChange, anchored at the location of the
+  // first change.
+  AtomicChange AC(*Result.SourceManager,
+  (*Transformations)[0].Range.getBegin());
   for (const auto  : *Transformations) {
 if (auto Err = AC.replace(*Result.SourceManager, T.Range, T.Replacement)) {
   Consumer(std::move(Err));

Modified: cfe/trunk/unittests/Tooling/TransformerTest.cpp
URL: 

[PATCH] D66652: [libTooling] Transformer: refine `SourceLocation` specified as anchor of changes.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D66652#1654585 , @gribozavr wrote:

> > So, I plan to rework this into two revisions: one to match 
> > https://reviews.llvm.org/D66676 (and keep the tests esssentially as they 
> > are) and one to add getRuleMatchLoc for future use.
>
> That SGTM.
>
> I don't have an opinion about whether they should be separate revisions or 
> not.


Reworked, per our discussion. I kept it in one revision, because I ended up 
useing the new function in this code (for error reporting).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66652



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

[PATCH] D68141: [Alignment][NFC] Remove AllocaInst::setAlignment(unsigned)

[Clement Courbet via Phabricator via cfe-commits]

courbet added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64StackTagging.cpp:461
 void AArch64StackTagging::alignAndPadAlloca(AllocaInfo ) {
-  unsigned NewAlignment = std::max(Info.AI->getAlignment(), kTagGranuleSize);
+  MaybeAlign NewAlignment(std::max(Info.AI->getAlignment(), kTagGranuleSize));
   Info.AI->setAlignment(NewAlignment);

This could be an align, with a static_assert that  `kTagGranuleSize` is > 0



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2909
   ".byval");
-  AI->setAlignment(Align);
+  AI->setAlignment(MaybeAlign(Align));
   Arg.replaceAllUsesWith(AI);

This is an `Align` because the `0` case has been checked just above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68141



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

[PATCH] D66652: [libTooling] Transformer: refine `SourceLocation` specified as anchor of changes.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 222177.
ymandel added a comment.
Herald added a subscriber: jfb.

reworked to follow same scheme as clang-tidy version, per discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66652

Files:
  clang/include/clang/Tooling/Refactoring/Transformer.h
  clang/lib/Tooling/Refactoring/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -710,6 +710,57 @@
   testRule(ruleStrlenSize(), Input, Expected);
 }
 
+// Tests that two changes in a single macro expansion do not lead to conflicts
+// in applying the changes.
+TEST_F(TransformerTest, TwoChangesInOneMacroExpansion) {
+  std::string Input = R"cc(
+#define PLUS(a,b) (a) + (b)
+int f() { return PLUS(3, 4); }
+  )cc";
+  std::string Expected = R"cc(
+#define PLUS(a,b) (a) + (b)
+int f() { return PLUS(LIT, LIT); }
+  )cc";
+
+  testRule(makeRule(integerLiteral(), change(text("LIT"))), Input, Expected);
+}
+
+// Tests case where the rule's match spans both source from the macro and its
+// arg, with the begin location (the "anchor") being the arg.
+TEST_F(TransformerTest, MatchSpansMacroTextButChangeDoesNot) {
+  std::string Input = R"cc(
+#define PLUS_ONE(a) a + 1
+int f() { return PLUS_ONE(3); }
+  )cc";
+  std::string Expected = R"cc(
+#define PLUS_ONE(a) a + 1
+int f() { return PLUS_ONE(LIT); }
+  )cc";
+
+  StringRef E = "expr";
+  testRule(makeRule(binaryOperator(hasLHS(expr().bind(E))),
+change(node(E), text("LIT"))),
+   Input, Expected);
+}
+
+// Tests case where the rule's match spans both source from the macro and its
+// arg, with the begin location (the "anchor") being inside the macro.
+TEST_F(TransformerTest, MatchSpansMacroTextButChangeDoesNotAnchoredInMacro) {
+  std::string Input = R"cc(
+#define PLUS_ONE(a) 1 + a
+int f() { return PLUS_ONE(3); }
+  )cc";
+  std::string Expected = R"cc(
+#define PLUS_ONE(a) 1 + a
+int f() { return PLUS_ONE(LIT); }
+  )cc";
+
+  StringRef E = "expr";
+  testRule(makeRule(binaryOperator(hasRHS(expr().bind(E))),
+change(node(E), text("LIT"))),
+   Input, Expected);
+}
+
 // No rewrite is applied when the changed text does not encompass the entirety
 // of the expanded text. That is, the edit would have to be applied to the
 // macro's definition to succeed and editing the expansion point would not
Index: clang/lib/Tooling/Refactoring/Transformer.cpp
===
--- clang/lib/Tooling/Refactoring/Transformer.cpp
+++ clang/lib/Tooling/Refactoring/Transformer.cpp
@@ -150,6 +150,21 @@
   return Ms[0];
 }
 
+SourceLocation tooling::detail::getRuleMatchLoc(const MatchResult ) {
+  auto  = Result.Nodes.getMap();
+  auto Root = NodesMap.find(RewriteRule::RootID);
+  assert(Root != NodesMap.end() && "Transformation failed: missing root node.");
+  llvm::Optional RootRange = getRangeForEdit(
+  CharSourceRange::getTokenRange(Root->second.getSourceRange()),
+  *Result.Context);
+  if (RootRange)
+return RootRange->getBegin();
+  // The match doesn't have a coherent range, so fall back to the expansion
+  // location as the "beginning" of the match.
+  return Result.SourceManager->getExpansionLoc(
+  Root->second.getSourceRange().getBegin());
+}
+
 // Finds the case that was "selected" -- that is, whose matcher triggered the
 // `MatchResult`.
 const RewriteRule::Case &
@@ -178,14 +193,6 @@
   if (Result.Context->getDiagnostics().hasErrorOccurred())
 return;
 
-  // Verify the existence and validity of the AST node that roots this rule.
-  auto  = Result.Nodes.getMap();
-  auto Root = NodesMap.find(RewriteRule::RootID);
-  assert(Root != NodesMap.end() && "Transformation failed: missing root node.");
-  SourceLocation RootLoc = Result.SourceManager->getExpansionLoc(
-  Root->second.getSourceRange().getBegin());
-  assert(RootLoc.isValid() && "Invalid location for Root node of match.");
-
   RewriteRule::Case Case = tooling::detail::findSelectedCase(Result, Rule);
   auto Transformations = tooling::detail::translateEdits(Result, Case.Edits);
   if (!Transformations) {
@@ -195,14 +202,16 @@
 
   if (Transformations->empty()) {
 // No rewrite applied (but no error encountered either).
-RootLoc.print(llvm::errs() << "note: skipping match at loc ",
-  *Result.SourceManager);
+detail::getRuleMatchLoc(Result).print(
+llvm::errs() << "note: skipping match at loc ", *Result.SourceManager);
 llvm::errs() << "\n";
 return;
   }
 
-  // Record the results in the AtomicChange.
-  AtomicChange AC(*Result.SourceManager, RootLoc);
+  // Record the results in the AtomicChange, anchored at the 

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

[Albert Astals Cid via Phabricator via cfe-commits]

tsdgeos added a comment.
Herald added a subscriber: wuzish.
Herald added a project: clang.

Would this warn with stuff like

  double borderWidth;
  [... code that doesn't use borderWidth ...]
  borderWidth = border->getWidth(); 
  [... code that reads borderWidth ...]


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D45444



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

[PATCH] D68141: [Alignment][NFC] Remove AllocaInst::setAlignment(unsigned)

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet created this revision.
gchatelet added a reviewer: courbet.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, eraman, 
nhaehnle, jvesely, arsenm, jholewinski.
Herald added projects: clang, LLVM.

This is patch is part of a series to introduce an Alignment type.
See this thread for context: 
http://lists.llvm.org/pipermail/llvm-dev/2019-July/133851.html
See this patch for the introduction of the type: https://reviews.llvm.org/D64790


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68141

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  llvm/include/llvm/IR/Instructions.h
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/Target/AArch64/AArch64StackTagging.cpp
  llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp
  llvm/lib/Target/NVPTX/NVPTXLowerArgs.cpp
  llvm/lib/Transforms/Coroutines/CoroFrame.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/lib/Transforms/Scalar/GVNHoist.cpp
  llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
  llvm/lib/Transforms/Utils/Local.cpp

Index: llvm/lib/Transforms/Utils/Local.cpp
===
--- llvm/lib/Transforms/Utils/Local.cpp
+++ llvm/lib/Transforms/Utils/Local.cpp
@@ -1154,7 +1154,7 @@
 // then don't round up. This avoids dynamic stack realignment.
 if (DL.exceedsNaturalStackAlignment(Align(PrefAlign)))
   return Alignment;
-AI->setAlignment(PrefAlign);
+AI->setAlignment(MaybeAlign(PrefAlign));
 return PrefAlign;
   }
 
Index: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
===
--- llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -895,7 +895,7 @@
   // If the destination wasn't sufficiently aligned then increase its alignment.
   if (!isDestSufficientlyAligned) {
 assert(isa(cpyDest) && "Can only increase alloca alignment!");
-cast(cpyDest)->setAlignment(srcAlign);
+cast(cpyDest)->setAlignment(MaybeAlign(srcAlign));
   }
 
   // Drop any cached information about the call, because we may have changed
Index: llvm/lib/Transforms/Scalar/GVNHoist.cpp
===
--- llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -900,8 +900,8 @@
   ++NumStoresRemoved;
 } else if (auto *ReplacementAlloca = dyn_cast(Repl)) {
   ReplacementAlloca->setAlignment(
-  std::max(ReplacementAlloca->getAlignment(),
-   cast(I)->getAlignment()));
+  MaybeAlign(std::max(ReplacementAlloca->getAlignment(),
+  cast(I)->getAlignment(;
 } else if (isa(Repl)) {
   ++NumCallsRemoved;
 }
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1184,7 +1184,7 @@
 uint64_t Size = getAllocaSizeInBytes(*AI);
 uint64_t AlignedSize = alignTo(Size, Mapping.getObjectAlignment());
 AI->setAlignment(
-std::max(AI->getAlignment(), Mapping.getObjectAlignment()));
+MaybeAlign(std::max(AI->getAlignment(), Mapping.getObjectAlignment(;
 if (Size != AlignedSize) {
   Type *AllocatedType = AI->getAllocatedType();
   if (AI->isArrayAllocation()) {
@@ -1197,7 +1197,7 @@
   auto *NewAI = new AllocaInst(
   TypeWithPadding, AI->getType()->getAddressSpace(), nullptr, "", AI);
   NewAI->takeName(AI);
-  NewAI->setAlignment(AI->getAlignment());
+  NewAI->setAlignment(MaybeAlign(AI->getAlignment()));
   NewAI->setUsedWithInAlloca(AI->isUsedWithInAlloca());
   NewAI->setSwiftError(AI->isSwiftError());
   NewAI->copyMetadata(*AI);
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2906,7 +2906,7 @@
   Ty, nullptr,
   (Arg.hasName() ? Arg.getName() : "Arg" + Twine(Arg.getArgNo())) +
   ".byval");
-  AI->setAlignment(Align);
+  AI->setAlignment(MaybeAlign(Align));
   Arg.replaceAllUsesWith(AI);
 
   uint64_t AllocSize = DL.getTypeAllocSize(Ty);
@@ -2941,7 +2941,7 @@
   }
   assert((ClRealignStack & (ClRealignStack - 1)) == 0);
   size_t FrameAlignment = std::max(L.FrameAlignment, 

[PATCH] D66834: Driver tests: set `--sysroot=""` to support clang with `DEFAULT_SYSROOT`

[Serge Pavlov via Phabricator via cfe-commits]

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

In D66834#1685921 , @broadwaylamb 
wrote:

> In D66834#1685260 , @sepavloff wrote:
>
> > In D66834#1653334 , @broadwaylamb 
> > wrote:
> >
> > > In D66834#1652756 , @compnerd 
> > > wrote:
> > >
> > > > I think that this is pretty easy to forget.  Fortunately, last argument 
> > > > wins.  Why not sink this into the `%clang` substitution in lit?  That 
> > > > ensures that we run with an empty sysroot and then when the test needs 
> > > > to adjust the sysroot, it can do so explicitly.
> > >
> > >
> > > I've just tried to do it, but unfortunately some tests are failing, for 
> > > example, `Driver/cc1-response-files.c`. The problem is that `%clang` is 
> > > expanded to `/path/to/clang --sysroot=`, but the succeeding flags (such 
> > > as `-cc1`) may be incompatible with `--sysroot`.
> >
> >
> > Does the issue manifests itself with `-cc1` only? We usually use 
> > `%clang_cc1` in such cases, so probably those tests require update.
>
>
> Some Driver tests take a list of arguments from a file (for example 
> `clang/test/Driver/cc1-response-files.c`), so we probably cannot use 
> `%clang_cc1` there


The obvious solution in this case is to implement support of an environment 
variable, say `SYSROOT` or `CLANG_SYSROOT`, which would override hard-coded 
path (but not that specified by --sysroot option). Unfortunately some 
developers don't like use of environment variables, as it creates 'secret' 
control channel, and such patch would have high chance to be rejected. So 
probably this is all we can do here.

This patch looks good for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66834



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

[PATCH] D63640: [clang] Improve Serialization/Imporing/Dumping of APValues

[Tyker via Phabricator via cfe-commits]

Tyker updated this revision to Diff 222169.
Tyker marked 3 inline comments as done.
Tyker added a comment.

made renamings


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

https://reviews.llvm.org/D63640

Files:
  clang/include/clang/AST/APValue.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/ASTImporter.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/PrettyPrinter.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/AST/ast-dump-color.cpp
  clang/test/ASTMerge/APValue/APValue.cpp

Index: clang/test/ASTMerge/APValue/APValue.cpp
===
--- /dev/null
+++ clang/test/ASTMerge/APValue/APValue.cpp
@@ -0,0 +1,209 @@
+// RUN: %clang_cc1 -std=gnu++2a -emit-pch %s -o %t.pch
+// RUN: %clang_cc1 -std=gnu++2a %s -DEMIT -ast-merge %t.pch -ast-dump-all | FileCheck %s
+
+#ifndef EMIT
+#define EMIT
+
+namespace Integer {
+
+constexpr int Unique_Int = int(6789);
+//CHECK:  VarDecl {{.*}} Unique_Int
+//CHECK-NEXT: ConstantExpr {{.*}} 'int' 6789
+
+constexpr __uint128_t Unique_Int128 = ((__uint128_t)0x75f17d6b3588f843 << 64) | 0xb13dea7c9c324e51;
+//CHECK:  VarDecl {{.*}} Unique_Int128 
+//CHECK-NEXT: ConstantExpr {{.*}} 'unsigned __int128' 156773562844924187900898496343692168785
+
+}
+
+namespace FloatingPoint {
+
+constexpr double Unique_Double = double(567890.67890);
+//CHECK:  VarDecl {{.*}} Unique_Double
+//CHECK-NEXT: ConstantExpr {{.*}} 'double' 5.678907e+05
+
+}
+
+// FIXME: Add test for FixedPoint, ComplexInt, ComplexFloat, AddrLabelDiff.
+
+namespace Struct {
+
+struct B {
+  int i;
+  double d;
+};
+
+constexpr B Basic_Struct = B{1, 0.7};
+//CHECK:  VarDecl {{.*}} Basic_Struct
+//CHECK-NEXT: ConstantExpr {{.*}} 'const Struct::B' {1, 7.00e-01}
+
+struct C {
+  int i = 9;
+};
+
+struct A : B {
+  int i;
+  double d;
+  C c;
+};
+
+constexpr A Advanced_Struct = A{Basic_Struct, 1, 79.789, {}};
+//CHECK:  VarDecl {{.*}} Advanced_Struct
+//CHECK-NEXT: ConstantExpr {{.*}} 'const Struct::A' {{[{][{]}}1, 7.00e-01}, 1, 7.978900e+01, {9}}
+
+}
+
+namespace Vector {
+
+using v4si = int __attribute__((__vector_size__(16)));
+
+constexpr v4si Vector_Int = (v4si){8, 2, 3};
+//CHECK:  VarDecl {{.*}} Vector_Int
+//CHECK-NEXT: ConstantExpr {{.*}} 'Vector::v4si':'__attribute__((__vector_size__(4 * sizeof(int int' {8, 2, 3, 0}
+
+}
+
+namespace Array {
+
+constexpr int Array_Int[] = {1, 2, 3, 4, 5, 6};
+//CHECK:  VarDecl {{.*}} Array_Int
+//CHECK-NEXT: ConstantExpr {{.*}} 'const int [6]' {1, 2, 3, 4, 5, 6}
+
+struct A {
+  int i = 789;
+  double d = 67890.09876;
+};
+
+constexpr A Array2_Struct[][3] = {{{}, {-45678, 9.8}, {9}}, {{}}};
+//CHECK:  VarDecl {{.*}} Array2_Struct
+//CHECK-NEXT: ConstantExpr {{.*}} 'Array::A const [2][3]' {{[{][{]}}{789, 6.789010e+04}, {-45678, 9.80e+00}, {9, 6.789010e+04}}, {{[{][{]}}789, 6.789010e+04}, {789, 6.789010e+04}, {789, 6.789010e+04}}}
+
+using v4si = int __attribute__((__vector_size__(16)));
+
+constexpr v4si Array_Vector[] = {{1, 2, 3, 4}, {4, 5, 6, 7}};
+//CHECK:  VarDecl {{.*}} Array_Vector
+//CHECK-NEXT: ConstantExpr {{.*}} 'const Array::v4si [2]' {{[{][{]}}1, 2, 3, 4}, {4, 5, 6, 7}}
+
+}
+
+namespace Union {
+
+struct A {
+  int i = 6789;
+  float f = 987.9876;
+};
+
+union U {
+  int i;
+  A a{567890, 9876.5678f};
+};
+
+constexpr U Unique_Union1 = U{0};
+//CHECK:  VarDecl {{.*}} Unique_Union
+//CHECK-NEXT: ConstantExpr {{.*}} 'const Union::U' {.i = 0}
+
+constexpr U Unique_Union2 = U{};
+//CHECK:  VarDecl {{.*}} Unique_Union
+//CHECK-NEXT: ConstantExpr {{.*}} 'const Union::U' {.a = {567890, 9.876567e+03}}
+
+}
+
+namespace MemberPointer{
+
+struct A {
+  struct B {
+struct C {
+  struct D {
+struct E {
+  struct F {
+struct G {
+  int i;
+};
+  };
+};
+  };
+};
+  };
+};
+
+constexpr auto MemberPointer1 = ::B::C::D::E::F::G::i;
+//CHECK:  VarDecl {{.*}} MemberPointer1
+//CHECK-NEXT: ConstantExpr {{.*}} 'int MemberPointer::A::B::C::D::E::F::G::*' ::i
+
+struct A1 {
+  struct B1 {
+int f() const {
+  return 0;
+}
+  };
+
+};
+
+constexpr auto MemberPointer2 = ::B1::f;
+//CHECK:  VarDecl {{.*}} MemberPointer2
+//CHECK-NEXT: ConstantExpr {{.*}} 'int (MemberPointer::A1::B1::*)() const' ::f
+
+}
+
+namespace std {
+  struct type_info;
+};
+
+namespace LValue {
+
+constexpr int LValueInt = 0;
+constexpr const int& ConstIntRef = LValueInt;
+//CHECK:  VarDecl {{.*}} ConstIntRef
+//CHECK-NEXT: ConstantExpr {{.*}} 'const int' lvalue 
+constexpr 

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

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

Szelethus added a comment.

It seems like this patch is diffed against your latest commit, not the master 
branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67706



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

[PATCH] D68028: [clang] Add no_builtin attribute

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D68028#1685912 , @gchatelet wrote:

> @tejohnson I believe this is the missing part for D67923 
> .


Thanks, yep I will take a closer look at the patch today.

> I'm unsure if we still need the `BitVector` at all in the `TLI` since it 
> could be a simple attribute lookup on the function.

If we didn't save the info on the TLI we would instead need to save the 
Function object in the TLI and query the attribute info off the Function on 
every lookup, which seems heavier weight. I think caching the info in that 
object for fast lookup is going to be better. And as noted in the comments 
there, we can replace it with the existing AvailableArray moved from the base 
Impl object into the TLI and remove the override bitvector once this goes in, 
we use these attributes to set the TLI info on construction, and we remove the 
clang code that sets the unavailability from the CodeGenOpts which will no 
longer be needed. If this patch goes in first I can just modify my TLI patch to 
do that all in one go. Maybe that is best...

> Do you see any problematic interactions with the inlining phase?

The inliner will need to be modified to respect the function attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68028



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

[PATCH] D66834: Driver tests: set `--sysroot=""` to support clang with `DEFAULT_SYSROOT`

[Sergej Jaskiewicz via Phabricator via cfe-commits]

broadwaylamb added a comment.

In D66834#1685260 , @sepavloff wrote:

> In D66834#1653334 , @broadwaylamb 
> wrote:
>
> > In D66834#1652756 , @compnerd 
> > wrote:
> >
> > > I think that this is pretty easy to forget.  Fortunately, last argument 
> > > wins.  Why not sink this into the `%clang` substitution in lit?  That 
> > > ensures that we run with an empty sysroot and then when the test needs to 
> > > adjust the sysroot, it can do so explicitly.
> >
> >
> > I've just tried to do it, but unfortunately some tests are failing, for 
> > example, `Driver/cc1-response-files.c`. The problem is that `%clang` is 
> > expanded to `/path/to/clang --sysroot=`, but the succeeding flags (such as 
> > `-cc1`) may be incompatible with `--sysroot`.
>
>
> Does the issue manifests itself with `-cc1` only? We usually use `%clang_cc1` 
> in such cases, so probably those tests require update.


Some Driver tests take a list of arguments from a file (for example 
`clang/test/Driver/cc1-response-files.c`), so we probably cannot use 
`%clang_cc1` there


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66834



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

[PATCH] D68028: [clang] Add no_builtin attribute

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet updated this revision to Diff 222163.
gchatelet added a comment.

- Update documentation and rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68028

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/no-builtin.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/no-builtin.c

Index: clang/test/Sema/no-builtin.c
===
--- /dev/null
+++ clang/test/Sema/no-builtin.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - -verify %s
+
+void foo() __attribute__((no_builtin)) {} // expected-error {{'no_builtin' attribute takes at least 1 argument}}
+
+void bar() __attribute__((no_builtin())) {} // expected-error {{'no_builtin' attribute takes at least 1 argument}}
+
+void invalid_builtin() __attribute__((no_builtin("not_a_builtin"))) {} // expected-error {{use of unknown builtin not_a_builtin}}
+
+int __attribute__((no_builtin("*"))) variable; // expected-warning {{'no_builtin' attribute only applies to functions}}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -74,6 +74,7 @@
 // CHECK-NEXT: NSConsumed (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: NSConsumesSelf (SubjectMatchRule_objc_method)
 // CHECK-NEXT: Naked (SubjectMatchRule_function)
+// CHECK-NEXT: NoBuiltin (SubjectMatchRule_function)
 // CHECK-NEXT: NoCommon (SubjectMatchRule_variable)
 // CHECK-NEXT: NoDebug (SubjectMatchRule_type_alias, SubjectMatchRule_hasType_functionType, SubjectMatchRule_objc_method, SubjectMatchRule_variable_not_is_parameter)
 // CHECK-NEXT: NoDestroy (SubjectMatchRule_variable)
Index: clang/test/CodeGen/no-builtin.c
===
--- /dev/null
+++ clang/test/CodeGen/no-builtin.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define void @foo_no_mempcy() #0
+void foo_no_mempcy() __attribute__((no_builtin("memcpy"))) {}
+
+// CHECK-LABEL: define void @foo_no_builtins() #1
+void foo_no_builtins() __attribute__((no_builtin("*"))) {}
+
+// CHECK-LABEL: define void @foo_no_mempcy_memset() #2
+void foo_no_mempcy_memset() __attribute__((no_builtin("memset", "memcpy"))) {}
+
+// CHECK-LABEL: define void @separate_attrs() #2
+void separate_attrs() __attribute__((no_builtin("memset"))) __attribute__((no_builtin("memcpy"))) {}
+
+// CHECK-LABEL: define void @wildcard_wins() #1
+void wildcard_wins() __attribute__((no_builtin("memset"))) __attribute__((no_builtin("*"))) __attribute__((no_builtin("memcpy"))) {}
+
+// CHECK: attributes #0 = {{{.*}}"no-builtin-memcpy"{{.*}}}
+// CHECK: attributes #1 = {{{.*}}"no-builtins"{{.*}}}
+// CHECK: attributes #2 = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -1068,6 +1068,62 @@
   S.Context, AL, Cond, Msg, DiagType, ArgDependent, cast(D)));
 }
 
+NoBuiltinAttr *
+Sema::mergeNoBuiltinAttr(Sema , Decl *D, const AttributeCommonInfo ,
+ llvm::ArrayRef FunctionNames) {
+  const StringRef Wildcard = "*";
+  llvm::SmallSetVector FunctionNamesSet;
+
+  // Insert previous NoBuiltin attributes.
+  if (D->hasAttr())
+for (StringRef FunctionName : D->getAttr()->functionNames())
+  FunctionNamesSet.insert(FunctionName);
+  // Insert new NoBuiltin attributes.
+  for (StringRef FunctionName : FunctionNames)
+FunctionNamesSet.insert(FunctionName);
+
+  // Wildcard is a superset of all builtins, we keep only this one.
+  if (FunctionNamesSet.count(Wildcard) > 0) {
+FunctionNamesSet.clear();
+FunctionNamesSet.insert(Wildcard);
+  }
+
+  assert((FunctionNamesSet.count(Wildcard) == 0) ||
+ (FunctionNamesSet.size() == 1) && "Wildcard must be on its own");
+
+  llvm::SmallVector UniqFunctionNames =
+  FunctionNamesSet.takeVector();
+  llvm::sort(UniqFunctionNames);
+
+  if (D->hasAttr())
+D->dropAttr();
+
+  return ::new (S.Context) NoBuiltinAttr(
+  S.Context, CI, UniqFunctionNames.data(), UniqFunctionNames.size());
+}
+
+static void handleNoBuiltin(Sema , Decl *D, const ParsedAttr ) {
+  if (!checkAttributeAtLeastNumArgs(S, AL, 1))
+return;
+
+  llvm::SmallVector FunctionNames;
+  for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
+StringRef FunctionName;
+SourceLocation LiteralLoc;
+if 

r373088 - [OpenCL] Pass LangOptions as const ref

[Sven van Haastregt via cfe-commits]

Author: svenvh
Date: Fri Sep 27 06:31:29 2019
New Revision: 373088

URL: http://llvm.org/viewvc/llvm-project?rev=373088=rev
Log:
[OpenCL] Pass LangOptions as const ref

Modified:
cfe/trunk/include/clang/Basic/OpenCLOptions.h

Modified: cfe/trunk/include/clang/Basic/OpenCLOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/OpenCLOptions.h?rev=373088=373087=373088=diff
==
--- cfe/trunk/include/clang/Basic/OpenCLOptions.h (original)
+++ cfe/trunk/include/clang/Basic/OpenCLOptions.h Fri Sep 27 06:31:29 2019
@@ -42,7 +42,7 @@ public:
 
   // Is supported as either an extension or an (optional) core feature for
   // OpenCL version \p CLVer.
-  bool isSupported(llvm::StringRef Ext, LangOptions LO) const {
+  bool isSupported(llvm::StringRef Ext, const LangOptions ) const {
 // In C++ mode all extensions should work at least as in v2.0.
 auto CLVer = LO.OpenCLCPlusPlus ? 200 : LO.OpenCLVersion;
 auto I = OptMap.find(Ext)->getValue();
@@ -51,7 +51,7 @@ public:
 
   // Is supported (optional) OpenCL core features for OpenCL version \p CLVer.
   // For supported extension, return false.
-  bool isSupportedCore(llvm::StringRef Ext, LangOptions LO) const {
+  bool isSupportedCore(llvm::StringRef Ext, const LangOptions ) const {
 // In C++ mode all extensions should work at least as in v2.0.
 auto CLVer = LO.OpenCLCPlusPlus ? 200 : LO.OpenCLVersion;
 auto I = OptMap.find(Ext)->getValue();
@@ -60,7 +60,7 @@ public:
 
   // Is supported OpenCL extension for OpenCL version \p CLVer.
   // For supported (optional) core feature, return false.
-  bool isSupportedExtension(llvm::StringRef Ext, LangOptions LO) const {
+  bool isSupportedExtension(llvm::StringRef Ext, const LangOptions ) const {
 // In C++ mode all extensions should work at least as in v2.0.
 auto CLVer = LO.OpenCLCPlusPlus ? 200 : LO.OpenCLVersion;
 auto I = OptMap.find(Ext)->getValue();


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

[PATCH] D68028: [clang] Add no_builtin attribute

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet added a comment.

@tejohnson I believe this is the missing part for D67923 
.

I'm unsure if we still need the `BitVector` at all in the `TLI` since it could 
be a simple attribute lookup on the function. Do you see any problematic 
interactions with the inlining phase?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68028



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

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

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

balazske updated this revision to Diff 222161.
balazske added a comment.

- Rename to DefaultOperatorNewAlignmentCheck.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67545

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp
  clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-mem57-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/cert-mem57-cpp.cpp

Index: clang-tools-extra/test/clang-tidy/cert-mem57-cpp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/cert-mem57-cpp.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s -std=c++17 cert-mem57-cpp %t
+
+namespace std {
+typedef __typeof(sizeof(int)) size_t;
+void *aligned_alloc(size_t, size_t);
+void free(void *);
+} // namespace std
+
+struct alignas(32) Vector1 {
+  char elems[32];
+};
+
+struct Vector2 {
+  char elems[32];
+};
+
+struct alignas(32) Vector3 {
+  char elems[32];
+  static void *operator new(std::size_t nbytes) noexcept(true) {
+return std::aligned_alloc(alignof(Vector3), nbytes);
+  }
+  static void operator delete(void *p) {
+std::free(p);
+  }
+};
+
+struct alignas(8) Vector4 {
+  char elems[32];
+};
+
+void f() {
+  auto *V1 = new Vector1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: allocation function returns a pointer with alignment 16 but the over-aligned type being allocated requires alignment 32 [cert-mem57-cpp]
+  auto *V2 = new Vector2;
+  auto *V3 = new Vector3;
+  auto *V4 = new Vector4;
+  auto *V1_Arr = new Vector1[2];
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: allocation function returns a pointer with alignment 16 but the over-aligned type being allocated requires alignment 32 [cert-mem57-cpp]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -98,6 +98,7 @@
cert-err61-cpp (redirects to misc-throw-by-value-catch-by-reference) 
cert-fio38-c (redirects to misc-non-copyable-objects) 
cert-flp30-c
+   cert-mem57-cpp
cert-msc30-c (redirects to cert-msc50-cpp) 
cert-msc32-c (redirects to cert-msc51-cpp) 
cert-msc50-cpp
Index: clang-tools-extra/docs/clang-tidy/checks/cert-mem57-cpp.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cert-mem57-cpp.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - cert-mem57-cpp
+
+cert-mem57-cpp
+==
+
+This check flags uses of default ``operator new`` where the type has extended
+alignment (an alignment greater than the fundamental alignment). (The default
+``operator new`` is guaranteed to provide the correct alignmment if the
+requested alignment is less or equal to the fundamental alignment).
+Only cases are detected (by design) where the ``operator new`` is not
+user-defined and is not a placement new (the reason is that in these cases we
+assume that the user provided the correct memory allocation).
+
+This check corresponds to the CERT C++ Coding Standard rule
+`MEM57-CPP. Avoid using default operator new for over-aligned types
+`_.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -73,6 +73,12 @@
   Finds instances where variables with static storage are initialized
   dynamically in header files.
 
+- New :doc:`cert-mem57-cpp
+  ` check.
+
+  Checks if an object of type with extended alignment is allocated by using
+  the default ``operator new``.
+
 - New :doc:`linuxkernel-must-use-errs
   ` check.
 
Index: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.h
@@ -0,0 +1,35 @@
+//===--- DefaultOperatorNewCheck.h - clang-tidy -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_DEFAULTOPERATORNEWALIGNMENTCHECK_H
+#define 

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

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Rebase and bail out on methods inside templated classes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65433

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.h
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -24,6 +24,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -33,6 +34,8 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
+#include 
 
 using ::testing::AllOf;
 using ::testing::HasSubstr;
@@ -554,7 +557,6 @@
   EXPECT_THAT(apply(" [[int a = 5;]] a++; "), StartsWith("fail"));
   // Don't extract return
   EXPECT_THAT(apply(" if(true) [[return;]] "), StartsWith("fail"));
-  
 }
 
 TEST_F(ExtractFunctionTest, FileTest) {
@@ -648,6 +650,171 @@
   EXPECT_THAT(apply(" for(;;) { [[while(1) break; break;]] }"),
   StartsWith("fail"));
 }
+
+TWEAK_TEST(DefineInline);
+TEST_F(DefineInlineTest, TriggersOnFunctionDecl) {
+  // Basic check for function body and signature.
+  EXPECT_AVAILABLE(R"cpp(
+  class Bar {
+void baz();
+  };
+
+  [[void [[Bar::[[b^a^z() [[{
+return;
+  }
+
+  void foo();
+  [[void [[f^o^o]]() [[{
+return;
+  }
+  )cpp");
+
+  EXPECT_UNAVAILABLE(R"cpp(
+  // Not a definition
+  vo^i[[d^ ^f]]^oo();
+
+  [[vo^id ]]foo[[()]] {[[
+[[(void)(5+3);
+return;]]
+  }]]
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, NoForwardDecl) {
+  Header = "void bar();";
+  EXPECT_UNAVAILABLE(R"cpp(
+  void bar() {
+return;
+  }
+  // FIXME: Generate a decl in the header.
+  void fo^o() {
+return;
+  })cpp");
+}
+
+TEST_F(DefineInlineTest, ReferencedDecls) {
+  EXPECT_AVAILABLE(R"cpp(
+void bar();
+void foo(int test);
+
+void fo^o(int baz) {
+  int x = 10;
+  bar();
+})cpp");
+
+  // Internal symbol usage.
+  Header = "void foo(int test);";
+  EXPECT_UNAVAILABLE(R"cpp(
+void bar();
+void fo^o(int baz) {
+  int x = 10;
+  bar();
+})cpp");
+
+  // Becomes available after making symbol visible.
+  Header = "void bar();" + Header;
+  EXPECT_AVAILABLE(R"cpp(
+void fo^o(int baz) {
+  int x = 10;
+  bar();
+})cpp");
+
+  // FIXME: Move declaration below bar to make it visible.
+  Header.clear();
+  EXPECT_UNAVAILABLE(R"cpp(
+void foo();
+void bar();
+
+void fo^o() {
+  bar();
+})cpp");
+
+  // Order doesn't matter within a class.
+  EXPECT_AVAILABLE(R"cpp(
+class Bar {
+  void foo();
+  void bar();
+};
+
+void Bar::fo^o() {
+  bar();
+})cpp");
+
+  // FIXME: Perform include insertion to make symbol visible.
+  ExtraFiles["a.h"] = "void bar();";
+  Header = "void foo(int test);";
+  EXPECT_UNAVAILABLE(R"cpp(
+#include "a.h"
+void fo^o(int baz) {
+  int x = 10;
+  bar();
+})cpp");
+}
+
+TEST_F(DefineInlineTest, TemplateSpec) {
+  EXPECT_UNAVAILABLE(R"cpp(
+template  void foo();
+template<> void foo();
+
+template<> void f^oo() {
+})cpp");
+  EXPECT_UNAVAILABLE(R"cpp(
+template  void foo();
+
+template<> void f^oo() {
+})cpp");
+  EXPECT_UNAVAILABLE(R"cpp(
+template  struct Foo { void foo(); };
+
+template  void Foo::f^oo() {
+})cpp");
+  EXPECT_AVAILABLE(R"cpp(
+template  void foo();
+void bar();
+template <> void foo();
+
+template<> void f^oo() {
+  bar();
+})cpp");
+}
+
+TEST_F(DefineInlineTest, CheckForCanonDecl) {
+  EXPECT_UNAVAILABLE(R"cpp(
+void foo();
+
+void bar() {}
+void f^oo() {
+  // This bar normally refers to the definition just above, but it is not
+  // visible from the forward declaration of foo.
+  bar();
+})cpp");
+  // Make it available with a forward decl.
+  EXPECT_AVAILABLE(R"cpp(
+void bar();
+void foo();
+
+void bar() {}
+void f^oo() {
+  bar();
+})cpp");
+}
+
+TEST_F(DefineInlineTest, UsingShadowDecls) {
+  EXPECT_UNAVAILABLE(R"cpp(
+  namespace ns1 { void foo(int); }
+  namespace ns2 { void foo(int*); }
+  template 
+  void bar();
+
+  using ns1::foo;
+  using ns2::foo;
+
+  template 
+  void b^ar() {
+foo(T());
+  })cpp");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/TweakTesting.h

[PATCH] D68024: [clangd] Implement GetEligiblePoints

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/SourceCode.cpp:997
+  // definitions as well. One might use a closing parantheses(")" followed by 
an
+  // opening brace "{" to trigger the start.
+  parseNamespaceEvents(Code, Style, [&](NamespaceEvent Event) {

hokein wrote:
> If I understand the FIXME well, so we will extend the API to parse functions, 
> so that we could get a more eligible point e.g. near the near the neighboring 
> declaration.
yes that's the idea. we'll try to provide as many valid insertion points as 
possible in the closest namespace.



Comment at: clang-tools-extra/clangd/SourceCode.h:273
+  /// definition.
+  std::vector EligiblePoints;
+};

hokein wrote:
> I'm curious how the caller use this, it seems that the caller has no/little 
> information to distinguish all the positions, even for a simple case where we 
> add a definition at the last eligible point (I assume just just using 
> `EligiblePoints.back()`).
for example a caller with access to AST and Index, could first look at the 
decls surrounding the target(fully qualified name), then check for their 
definition locations using index and find an eligible point between these two.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68024



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

[PATCH] D68137: [clangd] Handle template arguments in findExplicitReferences

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:576
+case TemplateArgument::Expression:
+  break; // Handled by visited functions.
+};

This comment is a bit unclear, I'll have to change it.
The idea is that this is handled by `VisitType` and `VisitExpression`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68137



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

[PATCH] D68137: [clangd] Handle template arguments in findExplicitReferences

[Ilya Biryukov via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68137

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -726,6 +726,36 @@
 }
 )cpp",
"0: targets = {I}\n"},
+  // Template template parameters.
+  {R"cpp(
+template  struct vector {};
+
+template  class TT, template class ...TP>
+void foo() {
+  $0^TT x;
+  $1^foo<$2^TT>();
+  $3^foo<$4^vector>()
+  $5^foo<$6^TP...>();
+}
+)cpp",
+   "0: targets = {TT}\n"
+   "1: targets = {foo}\n"
+   "2: targets = {TT}\n"
+   "3: targets = {foo}\n"
+   "4: targets = {vector}\n"
+   "5: targets = {foo}\n"
+   "6: targets = {TP}\n"},
+  // Non-type template parameters with declarations.
+  {R"cpp(
+int func();
+template  struct wrapper {};
+
+void foo() {
+  $0^wrapper<$1^func> w;
+}
+)cpp",
+   "0: targets = {wrapper<>}\n"
+   "1: targets = {func}\n"},
   };
 
   for (const auto  : Cases) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtVisitor.h"
+#include "clang/AST/TemplateBase.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeLocVisitor.h"
@@ -552,6 +553,31 @@
 return true;
   }
 
+  // We re-define Traverse*, since there's no corresponding Visit*.
+  bool TraverseTemplateArgumentLoc(TemplateArgumentLoc A) {
+switch (A.getArgument().getKind()) {
+case TemplateArgument::Template:
+case TemplateArgument::TemplateExpansion:
+  reportReference(ReferenceLoc{A.getTemplateQualifierLoc(),
+   A.getTemplateNameLoc(),
+   {A.getArgument()
+.getAsTemplateOrTemplatePattern()
+.getAsTemplateDecl()}});
+  break;
+case TemplateArgument::Declaration:
+  break; // FIXME: can this actually happen in TemplateArgumentLoc?
+case TemplateArgument::Integral:
+case TemplateArgument::Null:
+case TemplateArgument::NullPtr:
+  break; // no references.
+case TemplateArgument::Pack:
+case TemplateArgument::Type:
+case TemplateArgument::Expression:
+  break; // Handled by visited functions.
+};
+return RecursiveASTVisitor::TraverseTemplateArgumentLoc(A);
+  }
+
   bool VisitDecl(Decl *D) {
 visitNode(DynTypedNode::create(*D));
 return true;
@@ -610,15 +636,17 @@
 auto Ref = explicitReference(N);
 if (!Ref)
   return;
+reportReference(*Ref);
+  }
+
+  void reportReference(const ReferenceLoc ) {
 // Our promise is to return only references from the source code. If we lack
 // location information, skip these nodes.
 // Normally this should not happen in practice, unless there are bugs in the
 // traversals or users started the traversal at an implicit node.
-if (Ref->NameLoc.isInvalid()) {
-  dlog("invalid location at node {0}", nodeToString(N));
+if (Ref.NameLoc.isInvalid())
   return;
-}
-Out(*Ref);
+Out(Ref);
   }
 
   llvm::function_ref Out;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68024: [clangd] Implement GetEligiblePoints

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

The implementation looks good, just have some questions on the API.




Comment at: clang-tools-extra/clangd/SourceCode.cpp:997
+  // definitions as well. One might use a closing parantheses(")" followed by 
an
+  // opening brace "{" to trigger the start.
+  parseNamespaceEvents(Code, Style, [&](NamespaceEvent Event) {

If I understand the FIXME well, so we will extend the API to parse functions, 
so that we could get a more eligible point e.g. near the near the neighboring 
declaration.



Comment at: clang-tools-extra/clangd/SourceCode.h:270
+  /// It will be “a::b” for both carrot locations.
+  std::string CurrentNamespace;
+  /// Offsets into the code marking eligible points to insert a function

`Current` seems not clear enough, how about "MatchNamespace"?



Comment at: clang-tools-extra/clangd/SourceCode.h:273
+  /// definition.
+  std::vector EligiblePoints;
+};

I'm curious how the caller use this, it seems that the caller has no/little 
information to distinguish all the positions, even for a simple case where we 
add a definition at the last eligible point (I assume just just using 
`EligiblePoints.back()`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68024



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

[PATCH] D68132: clang-tidy: Don't repeat list of all checks in three places.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373082: clang-tidy: Dont repeat list of all checks in 
three places. (authored by nico, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68132?vs=222149=222153#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68132

Files:
  clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/trunk/clangd/CMakeLists.txt
  clang-tools-extra/trunk/clangd/ParsedAST.cpp

Index: clang-tools-extra/trunk/clangd/CMakeLists.txt
===
--- clang-tools-extra/trunk/clangd/CMakeLists.txt
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt
@@ -117,26 +117,6 @@
   clangSema
   clangSerialization
   clangTidy
-  clangTidyAndroidModule
-  clangTidyAbseilModule
-  clangTidyBoostModule
-  clangTidyBugproneModule
-  clangTidyCERTModule
-  clangTidyCppCoreGuidelinesModule
-  clangTidyDarwinModule
-  clangTidyFuchsiaModule
-  clangTidyGoogleModule
-  clangTidyHICPPModule
-  clangTidyLinuxKernelModule
-  clangTidyLLVMModule
-  clangTidyMiscModule
-  clangTidyModernizeModule
-  clangTidyObjCModule
-  clangTidyOpenMPModule
-  clangTidyPerformanceModule
-  clangTidyPortabilityModule
-  clangTidyReadabilityModule
-  clangTidyZirconModule
   clangTooling
   clangToolingCore
   clangToolingInclusions
@@ -144,6 +124,7 @@
   clangToolingSyntax
   ${LLVM_PTHREAD_LIB}
   ${CLANGD_ATOMIC_LIB}
+  ${ALL_CLANG_TIDY_CHECKS}
   )
 
 add_subdirectory(refactor/tweaks)
Index: clang-tools-extra/trunk/clangd/ParsedAST.cpp
===
--- clang-tools-extra/trunk/clangd/ParsedAST.cpp
+++ clang-tools-extra/trunk/clangd/ParsedAST.cpp
@@ -49,6 +49,11 @@
 #include 
 #include 
 
+// Force the linker to link in Clang-tidy modules.
+// clangd doesn't support the static analyzer.
+#define CLANG_TIDY_DISABLE_STATIC_ANALYZER_CHECKS
+#include "../clang-tidy/ClangTidyForceLinker.h"
+
 namespace clang {
 namespace clangd {
 namespace {
@@ -521,32 +526,4 @@
 }
 
 } // namespace clangd
-namespace tidy {
-// Force the linker to link in Clang-tidy modules.
-#define LINK_TIDY_MODULE(X)\
-  extern volatile int X##ModuleAnchorSource;   \
-  static int LLVM_ATTRIBUTE_UNUSED X##ModuleAnchorDestination =\
-  X##ModuleAnchorSource
-LINK_TIDY_MODULE(Abseil);
-LINK_TIDY_MODULE(Android);
-LINK_TIDY_MODULE(Boost);
-LINK_TIDY_MODULE(Bugprone);
-LINK_TIDY_MODULE(CERT);
-LINK_TIDY_MODULE(CppCoreGuidelines);
-LINK_TIDY_MODULE(Fuchsia);
-LINK_TIDY_MODULE(Google);
-LINK_TIDY_MODULE(HICPP);
-LINK_TIDY_MODULE(LinuxKernel);
-LINK_TIDY_MODULE(LLVM);
-LINK_TIDY_MODULE(Misc);
-LINK_TIDY_MODULE(Modernize);
-// LINK_TIDY_MODULE(MPI); // clangd doesn't support static analyzer.
-LINK_TIDY_MODULE(ObjC);
-LINK_TIDY_MODULE(OpenMP);
-LINK_TIDY_MODULE(Performance);
-LINK_TIDY_MODULE(Portability);
-LINK_TIDY_MODULE(Readability);
-LINK_TIDY_MODULE(Zircon);
-#undef LINK_TIDY_MODULE
-} // namespace tidy
 } // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h
@@ -80,7 +80,8 @@
 static int LLVM_ATTRIBUTE_UNUSED ModernizeModuleAnchorDestination =
 ModernizeModuleAnchorSource;
 
-#if CLANG_ENABLE_STATIC_ANALYZER
+#if CLANG_ENABLE_STATIC_ANALYZER &&\
+!defined(CLANG_TIDY_DISABLE_STATIC_ANALYZER_CHECKS)
 // This anchor is used to force the linker to link the MPIModule.
 extern volatile int MPIModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED MPIModuleAnchorDestination =
Index: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
@@ -37,6 +37,8 @@
   )
 endif()
 
+# Checks.
+# If you add a check, also add it to ClangTidyForceLinker.h in this directory.
 add_subdirectory(android)
 add_subdirectory(abseil)
 add_subdirectory(boost)
@@ -57,9 +59,38 @@
 add_subdirectory(objc)
 add_subdirectory(openmp)
 add_subdirectory(performance)
-add_subdirectory(plugin)
 add_subdirectory(portability)
 add_subdirectory(readability)
+add_subdirectory(zircon)
+set(ALL_CLANG_TIDY_CHECKS
+  clangTidyAndroidModule
+  clangTidyAbseilModule
+  clangTidyBoostModule
+  clangTidyBugproneModule
+  clangTidyCERTModule
+  clangTidyCppCoreGuidelinesModule
+  

[PATCH] D68132: clang-tidy: Don't repeat list of all checks in three places.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks! I'll land this, but beanz, if the cmake setup here is weird please 
shout and I'll try to make it less weird. We have a few other instances of 
PARENT_SCOPE variables, so maybe this is how cmake likes to do "group of 
libraries".


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

https://reviews.llvm.org/D68132



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

[clang-tools-extra] r373082 - clang-tidy: Don't repeat list of all checks in three places.

[Nico Weber via cfe-commits]

Author: nico
Date: Fri Sep 27 05:56:14 2019
New Revision: 373082

URL: http://llvm.org/viewvc/llvm-project?rev=373082=rev
Log:
clang-tidy: Don't repeat list of all checks in three places.

Instead, put all checks in a cmake variable and reference this.

Also, make clangd use the the ClangTidyForceLinker.h header instead
of duplicating the list of modules -- the duplicate copy was missing
the new "darwin" checker added in r373065.

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

Modified:
clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h
clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/ParsedAST.cpp

Modified: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/CMakeLists.txt?rev=373082=373081=373082=diff
==
--- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt Fri Sep 27 05:56:14 2019
@@ -37,6 +37,8 @@ if(CLANG_ENABLE_STATIC_ANALYZER)
   )
 endif()
 
+# Checks.
+# If you add a check, also add it to ClangTidyForceLinker.h in this directory.
 add_subdirectory(android)
 add_subdirectory(abseil)
 add_subdirectory(boost)
@@ -57,9 +59,38 @@ endif()
 add_subdirectory(objc)
 add_subdirectory(openmp)
 add_subdirectory(performance)
-add_subdirectory(plugin)
 add_subdirectory(portability)
 add_subdirectory(readability)
+add_subdirectory(zircon)
+set(ALL_CLANG_TIDY_CHECKS
+  clangTidyAndroidModule
+  clangTidyAbseilModule
+  clangTidyBoostModule
+  clangTidyBugproneModule
+  clangTidyCERTModule
+  clangTidyCppCoreGuidelinesModule
+  clangTidyDarwinModule
+  clangTidyFuchsiaModule
+  clangTidyGoogleModule
+  clangTidyHICPPModule
+  clangTidyLinuxKernelModule
+  clangTidyLLVMModule
+  clangTidyMiscModule
+  clangTidyModernizeModule
+  clangTidyObjCModule
+  clangTidyOpenMPModule
+  clangTidyPerformanceModule
+  clangTidyPortabilityModule
+  clangTidyReadabilityModule
+  clangTidyZirconModule
+  )
+if(CLANG_ENABLE_STATIC_ANALYZER)
+  list(APPEND ALL_CLANG_TIDY_CHECKS clangTidyMPIModule)
+endif()
+set(ALL_CLANG_TIDY_CHECKS ${ALL_CLANG_TIDY_CHECKS} PARENT_SCOPE)
+
+# Other subtargets. These may reference ALL_CLANG_TIDY_CHECKS
+# and must be below its definition.
+add_subdirectory(plugin)
 add_subdirectory(tool)
 add_subdirectory(utils)
-add_subdirectory(zircon)

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h?rev=373082=373081=373082=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h Fri Sep 27 
05:56:14 2019
@@ -80,7 +80,8 @@ extern volatile int ModernizeModuleAncho
 static int LLVM_ATTRIBUTE_UNUSED ModernizeModuleAnchorDestination =
 ModernizeModuleAnchorSource;
 
-#if CLANG_ENABLE_STATIC_ANALYZER
+#if CLANG_ENABLE_STATIC_ANALYZER &&
\
+!defined(CLANG_TIDY_DISABLE_STATIC_ANALYZER_CHECKS)
 // This anchor is used to force the linker to link the MPIModule.
 extern volatile int MPIModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED MPIModuleAnchorDestination =

Modified: clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt?rev=373082=373081=373082=diff
==
--- clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt Fri Sep 27 
05:56:14 2019
@@ -8,31 +8,6 @@ add_clang_library(clangTidyPlugin
   clangFrontend
   clangSema
   clangTidy
-  clangTidyAbseilModule
-  clangTidyAndroidModule
-  clangTidyBoostModule
-  clangTidyBugproneModule
-  clangTidyCERTModule
-  clangTidyCppCoreGuidelinesModule
-  clangTidyDarwinModule
-  clangTidyFuchsiaModule
-  clangTidyGoogleModule
-  clangTidyHICPPModule
-  clangTidyLinuxKernelModule
-  clangTidyLLVMModule
-  clangTidyMiscModule
-  clangTidyModernizeModule
-  clangTidyObjCModule
-  clangTidyOpenMPModule
-  clangTidyPerformanceModule
-  clangTidyPortabilityModule
-  clangTidyReadabilityModule
-  clangTidyZirconModule
   clangTooling
+  ${ALL_CLANG_TIDY_CHECKS}
   )
-
-if(CLANG_ENABLE_STATIC_ANALYZER)
-  target_link_libraries(clangTidyPlugin PRIVATE
-clangTidyMPIModule
-  )
-endif()

Modified: clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
URL: 

[PATCH] D68132: clang-tidy: Don't repeat list of all checks in three places.

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added reviewers: beanz, mwyman, gribozavr.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, 
ilya-biryukov, mgorny, srhines.
Herald added a reviewer: jdoerfert.

Instead, put all checks in a cmake variable and reference this.

Also, make clangd use the the ClangTidyForceLinker.h header instead
of duplicating the list of modules -- the duplicate copy was missing
the new "darwin" checker added in r373065.


https://reviews.llvm.org/D68132

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/plugin/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ParsedAST.cpp

Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -49,6 +49,11 @@
 #include 
 #include 
 
+// Force the linker to link in Clang-tidy modules.
+// clangd doesn't support the static analyzer.
+#define CLANG_TIDY_DISABLE_STATIC_ANALYZER_CHECKS
+#include "../clang-tidy/ClangTidyForceLinker.h"
+
 namespace clang {
 namespace clangd {
 namespace {
@@ -521,32 +526,4 @@
 }
 
 } // namespace clangd
-namespace tidy {
-// Force the linker to link in Clang-tidy modules.
-#define LINK_TIDY_MODULE(X)\
-  extern volatile int X##ModuleAnchorSource;   \
-  static int LLVM_ATTRIBUTE_UNUSED X##ModuleAnchorDestination =\
-  X##ModuleAnchorSource
-LINK_TIDY_MODULE(Abseil);
-LINK_TIDY_MODULE(Android);
-LINK_TIDY_MODULE(Boost);
-LINK_TIDY_MODULE(Bugprone);
-LINK_TIDY_MODULE(CERT);
-LINK_TIDY_MODULE(CppCoreGuidelines);
-LINK_TIDY_MODULE(Fuchsia);
-LINK_TIDY_MODULE(Google);
-LINK_TIDY_MODULE(HICPP);
-LINK_TIDY_MODULE(LinuxKernel);
-LINK_TIDY_MODULE(LLVM);
-LINK_TIDY_MODULE(Misc);
-LINK_TIDY_MODULE(Modernize);
-// LINK_TIDY_MODULE(MPI); // clangd doesn't support static analyzer.
-LINK_TIDY_MODULE(ObjC);
-LINK_TIDY_MODULE(OpenMP);
-LINK_TIDY_MODULE(Performance);
-LINK_TIDY_MODULE(Portability);
-LINK_TIDY_MODULE(Readability);
-LINK_TIDY_MODULE(Zircon);
-#undef LINK_TIDY_MODULE
-} // namespace tidy
 } // namespace clang
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -117,26 +117,6 @@
   clangSema
   clangSerialization
   clangTidy
-  clangTidyAndroidModule
-  clangTidyAbseilModule
-  clangTidyBoostModule
-  clangTidyBugproneModule
-  clangTidyCERTModule
-  clangTidyCppCoreGuidelinesModule
-  clangTidyDarwinModule
-  clangTidyFuchsiaModule
-  clangTidyGoogleModule
-  clangTidyHICPPModule
-  clangTidyLinuxKernelModule
-  clangTidyLLVMModule
-  clangTidyMiscModule
-  clangTidyModernizeModule
-  clangTidyObjCModule
-  clangTidyOpenMPModule
-  clangTidyPerformanceModule
-  clangTidyPortabilityModule
-  clangTidyReadabilityModule
-  clangTidyZirconModule
   clangTooling
   clangToolingCore
   clangToolingInclusions
@@ -144,6 +124,7 @@
   clangToolingSyntax
   ${LLVM_PTHREAD_LIB}
   ${CLANGD_ATOMIC_LIB}
+  ${ALL_CLANG_TIDY_CHECKS}
   )
 
 add_subdirectory(refactor/tweaks)
Index: clang-tools-extra/clang-tidy/tool/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/tool/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/tool/CMakeLists.txt
@@ -17,35 +17,11 @@
   clangASTMatchers
   clangBasic
   clangTidy
-  clangTidyAndroidModule
-  clangTidyAbseilModule
-  clangTidyBoostModule
-  clangTidyBugproneModule
-  clangTidyCERTModule
-  clangTidyCppCoreGuidelinesModule
-  clangTidyDarwinModule
-  clangTidyFuchsiaModule
-  clangTidyGoogleModule
-  clangTidyHICPPModule
-  clangTidyLinuxKernelModule
-  clangTidyLLVMModule
-  clangTidyMiscModule
-  clangTidyModernizeModule
-  clangTidyObjCModule
-  clangTidyOpenMPModule
-  clangTidyPerformanceModule
-  clangTidyPortabilityModule
-  clangTidyReadabilityModule
-  clangTidyZirconModule
   clangTooling
   clangToolingCore
+  ${ALL_CLANG_TIDY_CHECKS}
   )
 
-if(CLANG_ENABLE_STATIC_ANALYZER)
-  target_link_libraries(clang-tidy PRIVATE
-clangTidyMPIModule
-  )
-endif()
 
 install(PROGRAMS clang-tidy-diff.py
   DESTINATION share/clang
Index: clang-tools-extra/clang-tidy/plugin/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/plugin/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/plugin/CMakeLists.txt
@@ -8,31 +8,6 @@
   clangFrontend
   clangSema
   clangTidy
-  clangTidyAbseilModule
-  clangTidyAndroidModule
-  clangTidyBoostModule
-  clangTidyBugproneModule
-  clangTidyCERTModule
-  clangTidyCppCoreGuidelinesModule
-  clangTidyDarwinModule
-  clangTidyFuchsiaModule
-  clangTidyGoogleModule
-  

[clang-tools-extra] r373079 - [clangd] Remove an unrelated comment, NFC.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Fri Sep 27 05:32:19 2019
New Revision: 373079

URL: http://llvm.org/viewvc/llvm-project?rev=373079=rev
Log:
[clangd] Remove an unrelated comment, NFC.

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

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp?rev=373079=373078=373079=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp Fri Sep 27 05:32:19 
2019
@@ -112,7 +112,6 @@ public:
 // visitor.
 for (const auto  : AST.getMacros().Ranges)
   Tokens.push_back({HighlightingKind::Macro, M});
-// addToken(Loc, HighlightingKind::Macro);
 // Initializer lists can give duplicates of tokens, therefore all tokens
 // must be deduplicated.
 llvm::sort(Tokens);


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

[PATCH] D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373078: [clang] [AST] Treat inline gnu_inline 
the same way as extern inline… (authored by mstorsjo, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67414?vs=221086=222145#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67414

Files:
  cfe/trunk/docs/ReleaseNotes.rst
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/AST/Decl.cpp
  cfe/trunk/lib/Sema/SemaDeclAttr.cpp
  cfe/trunk/test/CodeGen/inline.c
  cfe/trunk/test/SemaCUDA/gnu-inline.cu
  cfe/trunk/test/SemaCXX/gnu_inline.cpp
  cfe/trunk/test/SemaCXX/undefined-inline.cpp

Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3008,6 +3008,10 @@
   "'gnu_inline' attribute requires function to be marked 'inline',"
   " attribute ignored">,
   InGroup;
+def warn_gnu_inline_cplusplus_without_extern : Warning<
+  "'gnu_inline' attribute without 'extern' in C++ treated as externally"
+  " available, this changed in Clang 10">,
+  InGroup>;
 def err_attribute_vecreturn_only_vector_member : Error<
   "the vecreturn attribute can only be used on a class or structure with one member, which must be a vector">;
 def err_attribute_vecreturn_only_pod_record : Error<
Index: cfe/trunk/test/SemaCUDA/gnu-inline.cu
===
--- cfe/trunk/test/SemaCUDA/gnu-inline.cu
+++ cfe/trunk/test/SemaCUDA/gnu-inline.cu
@@ -7,4 +7,4 @@
 // Check that we can handle gnu_inline functions when compiling in CUDA mode.
 
 void foo();
-inline __attribute__((gnu_inline)) void bar() { foo(); }
+extern inline __attribute__((gnu_inline)) void bar() { foo(); }
Index: cfe/trunk/test/SemaCXX/gnu_inline.cpp
===
--- cfe/trunk/test/SemaCXX/gnu_inline.cpp
+++ cfe/trunk/test/SemaCXX/gnu_inline.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+extern inline
+__attribute__((__gnu_inline__))
+void gnu_inline1() {}
+
+inline
+__attribute__((__gnu_inline__)) // expected-warning {{'gnu_inline' attribute without 'extern' in C++ treated as externally available, this changed in Clang 10}}
+void gnu_inline2() {}
Index: cfe/trunk/test/SemaCXX/undefined-inline.cpp
===
--- cfe/trunk/test/SemaCXX/undefined-inline.cpp
+++ cfe/trunk/test/SemaCXX/undefined-inline.cpp
@@ -40,20 +40,20 @@
 }
 
 namespace test8 {
-  inline void foo() __attribute__((gnu_inline));
+  inline void foo() __attribute__((gnu_inline)); // expected-warning {{'gnu_inline' attribute without 'extern' in C++ treated as externally available, this changed in Clang 10}}
   void test() { foo(); }
 }
 
 namespace test9 {
   void foo();
   void test() { foo(); }
-  inline void foo() __attribute__((gnu_inline));
+  inline void foo() __attribute__((gnu_inline)); // expected-warning {{'gnu_inline' attribute without 'extern' in C++ treated as externally available, this changed in Clang 10}}
 }
 
 namespace test10 {
   inline void foo();
   void test() { foo(); }
-  inline void foo() __attribute__((gnu_inline));
+  inline void foo() __attribute__((gnu_inline)); // expected-warning {{'gnu_inline' attribute without 'extern' in C++ treated as externally available, this changed in Clang 10}}
 }
 
 namespace test11 {
Index: cfe/trunk/test/CodeGen/inline.c
===
--- cfe/trunk/test/CodeGen/inline.c
+++ cfe/trunk/test/CodeGen/inline.c
@@ -52,7 +52,7 @@
 // CHECK3-LABEL: define i32 @_Z3barv()
 // CHECK3-LABEL: define linkonce_odr i32 @_Z3foov()
 // CHECK3-NOT: unreferenced
-// CHECK3-LABEL: define void @_Z10gnu_inlinev()
+// CHECK3-LABEL: define available_externally void @_Z10gnu_inlinev()
 // CHECK3-LABEL: define available_externally void @_Z13gnu_ei_inlinev()
 // CHECK3-NOT: @_Z5testCv
 // CHECK3-LABEL: define linkonce_odr i32 @_Z2eiv()
@@ -85,6 +85,7 @@
 extern __inline void unreferenced2() {}
 
 __inline __attribute((__gnu_inline__)) void gnu_inline() {}
+void (*P1)() = gnu_inline;
 
 // PR3988
 extern __inline __attribute__((gnu_inline)) void gnu_ei_inline() {}
Index: cfe/trunk/lib/AST/Decl.cpp
===
--- cfe/trunk/lib/AST/Decl.cpp
+++ cfe/trunk/lib/AST/Decl.cpp
@@ -3261,6 +3261,9 @@
   return true;
   }
 
+  if (Context.getLangOpts().CPlusPlus)
+return false;
+
   if (Context.getLangOpts().GNUInline || hasAttr()) {
 // With GNU inlining, a declaration with 'inline' but not 'extern', forces
 // an externally visible definition.
@@ -3289,9 +3292,6 @@
 return 

r373078 - [clang] [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Fri Sep 27 05:25:19 2019
New Revision: 373078

URL: http://llvm.org/viewvc/llvm-project?rev=373078=rev
Log:
[clang] [AST] Treat "inline gnu_inline" the same way as "extern inline 
gnu_inline" in C++ mode

This matches how GCC handles it, see e.g. https://gcc.godbolt.org/z/HPplnl.
GCC documents the gnu_inline attribute with "In C++, this attribute does
not depend on extern in any way, but it still requires the inline keyword
to enable its special behavior."

The previous behaviour of gnu_inline in C++, without the extern
keyword, can be traced back to the original commit that added
support for gnu_inline, SVN r69045.

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

Added:
cfe/trunk/test/SemaCXX/gnu_inline.cpp
Modified:
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/CodeGen/inline.c
cfe/trunk/test/SemaCUDA/gnu-inline.cu
cfe/trunk/test/SemaCXX/undefined-inline.cpp

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=373078=373077=373078=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Fri Sep 27 05:25:19 2019
@@ -128,7 +128,10 @@ C11 Feature Support
 C++ Language Changes in Clang
 -
 
-- ...
+- The behaviour of the `gnu_inline` attribute now matches GCC, for cases
+  where used without the `extern` keyword. As this is a change compared to
+  how it behaved in previous Clang versions, a warning is emitted for this
+  combination.
 
 C++1z Feature Support
 ^

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=373078=373077=373078=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Sep 27 05:25:19 
2019
@@ -3008,6 +3008,10 @@ def warn_gnu_inline_attribute_requires_i
   "'gnu_inline' attribute requires function to be marked 'inline',"
   " attribute ignored">,
   InGroup;
+def warn_gnu_inline_cplusplus_without_extern : Warning<
+  "'gnu_inline' attribute without 'extern' in C++ treated as externally"
+  " available, this changed in Clang 10">,
+  InGroup>;
 def err_attribute_vecreturn_only_vector_member : Error<
   "the vecreturn attribute can only be used on a class or structure with one 
member, which must be a vector">;
 def err_attribute_vecreturn_only_pod_record : Error<

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=373078=373077=373078=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Fri Sep 27 05:25:19 2019
@@ -3261,6 +3261,9 @@ bool FunctionDecl::doesDeclarationForceE
   return true;
   }
 
+  if (Context.getLangOpts().CPlusPlus)
+return false;
+
   if (Context.getLangOpts().GNUInline || hasAttr()) {
 // With GNU inlining, a declaration with 'inline' but not 'extern', forces
 // an externally visible definition.
@@ -3289,9 +3292,6 @@ bool FunctionDecl::doesDeclarationForceE
 return FoundBody;
   }
 
-  if (Context.getLangOpts().CPlusPlus)
-return false;
-
   // C99 6.7.4p6:
   //   [...] If all of the file scope declarations for a function in a
   //   translation unit include the inline function specifier without extern,
@@ -3371,6 +3371,8 @@ bool FunctionDecl::isInlineDefinitionExt
 // If it's not the case that both 'inline' and 'extern' are
 // specified on the definition, then this inline definition is
 // externally visible.
+if (Context.getLangOpts().CPlusPlus)
+  return false;
 if (!(isInlineSpecified() && getStorageClass() == SC_Extern))
   return true;
 

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=373078=373077=373078=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Sep 27 05:25:19 2019
@@ -4255,6 +4255,9 @@ static void handleGNUInlineAttr(Sema ,
 return;
   }
 
+  if (S.LangOpts.CPlusPlus && Fn->getStorageClass() != SC_Extern)
+S.Diag(AL.getLoc(), diag::warn_gnu_inline_cplusplus_without_extern);
+
   D->addAttr(::new (S.Context) GNUInlineAttr(S.Context, AL));
 }
 

Modified: cfe/trunk/test/CodeGen/inline.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/inline.c?rev=373078=373077=373078=diff

[PATCH] D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154)

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:181
+  addToken(E->getMemberLoc(), E->getQualifier()
+  ? HighlightingKind::StaticField
+  : HighlightingKind::Field);

nridge wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > nridge wrote:
> > > > hokein wrote:
> > > > > This could be member functions, a case is like
> > > > > 
> > > > > ```
> > > > > template
> > > > > class Foo {
> > > > > public:
> > > > >   void foo() {
> > > > > this->foo();
> > > > >   }
> > > > > };
> > > > > ```
> > > > Thanks for the example.
> > > > 
> > > > Do you have a suggestion for how to discriminate this case? To me, it 
> > > > would seem logical to do it based on syntax (highlight as a member 
> > > > function if the expression forms the function name of a function call 
> > > > expression). That would require navigating from the expression to its 
> > > > parent node. Is there a way to do that?
> > > There is no way to do this in C++.
> > > Even if the name is followed by a pair of parenthese, this could either 
> > > be a field with overloaded `operator()` (e.g. a `std::function 
> > > field`) or a function with the same name.
> > > 
> > > It's much better to pick a separate highlighting kind for dependent 
> > > names, this follows the actual semantics of C++.
> > +1, I think we should just highlight them as a dependent type.
> Of course, any attempt to disambiguate between a member function and a field 
> would be heuristic only. I figured that would be better than nothing. But if 
> you prefer using a separate highlighting for dependent names that resolve to 
> a function or a variable, we could do that.
> 
> (Hokein, I assume you don't actually mean using the dependent *type* 
> highlighting. Using a type highlighting for something we know is not a type, 
> but rather a function or variable, would be rather confusing.)
> But if you prefer using a separate highlighting for dependent names that 
> resolve to a function or a variable, we could do that.

I'd suggest we'd better follow this. The heuristic could fail in some cases.  
For these cases, the wrong highlighting'd confuse users too.

> (Hokein, I assume you don't actually mean using the dependent *type* 
> highlighting. Using a type highlighting for something we know is not a type, 
> but rather a function or variable, would be rather confusing.)

sorry for the confusion, I meant for anything that could not resolve to a 
specific declaration (e.g. `CXXDependentScopeMemberExpr`, 
`UnresolvedLookupExpr`), we use the new `dependent` highlighting kind.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67901



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

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

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

balazske updated this revision to Diff 222138.
balazske added a comment.

Using CallDescriptionMap.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67706

Files:
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -57,38 +57,53 @@
 
 class StreamChecker : public Checker {
-  CallDescription CD_fopen, CD_tmpfile, CD_fclose, CD_fread, CD_fwrite,
-  CD_fseek, CD_ftell, CD_rewind, CD_fgetpos, CD_fsetpos, CD_clearerr,
-  CD_feof, CD_ferror, CD_fileno;
   mutable std::unique_ptr BT_nullfp, BT_illegalwhence,
   BT_doubleclose, BT_ResourceLeak;
 
 public:
-  StreamChecker()
-  : CD_fopen("fopen"), CD_tmpfile("tmpfile"), CD_fclose("fclose", 1),
-CD_fread("fread", 4), CD_fwrite("fwrite", 4), CD_fseek("fseek", 3),
-CD_ftell("ftell", 1), CD_rewind("rewind", 1), CD_fgetpos("fgetpos", 2),
-CD_fsetpos("fsetpos", 2), CD_clearerr("clearerr", 1),
-CD_feof("feof", 1), CD_ferror("ferror", 1), CD_fileno("fileno", 1) {}
+  StreamChecker() = default;
 
   bool evalCall(const CallEvent , CheckerContext ) const;
   void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
 
 private:
-  void Fopen(CheckerContext , const CallExpr *CE) const;
-  void Tmpfile(CheckerContext , const CallExpr *CE) const;
-  void Fclose(CheckerContext , const CallExpr *CE) const;
-  void Fread(CheckerContext , const CallExpr *CE) const;
-  void Fwrite(CheckerContext , const CallExpr *CE) const;
-  void Fseek(CheckerContext , const CallExpr *CE) const;
-  void Ftell(CheckerContext , const CallExpr *CE) const;
-  void Rewind(CheckerContext , const CallExpr *CE) const;
-  void Fgetpos(CheckerContext , const CallExpr *CE) const;
-  void Fsetpos(CheckerContext , const CallExpr *CE) const;
-  void Clearerr(CheckerContext , const CallExpr *CE) const;
-  void Feof(CheckerContext , const CallExpr *CE) const;
-  void Ferror(CheckerContext , const CallExpr *CE) const;
-  void Fileno(CheckerContext , const CallExpr *CE) const;
+  typedef void (StreamChecker::*FnCheck)(CheckerContext &,
+ const CallExpr *) const;
+
+  CallDescriptionMap Callbacks = {
+  {{CDF_MaybeBuiltin, "fopen"}, ::evalFopen},
+  {{CDF_MaybeBuiltin, "tmpfile"}, ::evalTmpfile},
+  {{CDF_MaybeBuiltin, "fclose", 1}, ::evalFclose},
+  {{CDF_MaybeBuiltin, "fread", 4}, ::evalFread},
+  {{CDF_MaybeBuiltin, "fwrite", 4}, ::evalFwrite},
+  {{CDF_MaybeBuiltin, "fseek", 3}, ::evalFseek},
+  {{CDF_MaybeBuiltin, "ftell", 1}, ::evalFtell},
+  {{CDF_MaybeBuiltin, "rewind", 1}, ::evalRewind},
+  {{CDF_MaybeBuiltin, "fgetpos", 2}, ::evalFgetpos},
+  {{CDF_MaybeBuiltin, "fsetpos", 2}, ::evalFsetpos},
+  {{CDF_MaybeBuiltin, "clearerr", 1}, ::evalClearerr},
+  {{CDF_MaybeBuiltin, "feof", 1}, ::evalFeof},
+  {{CDF_MaybeBuiltin, "ferror", 1}, ::evalFerror},
+  {{CDF_MaybeBuiltin, "fileno", 1}, ::evalFileno},
+  };
+
+  FnCheck identifyCall(const CallEvent , CheckerContext ,
+   const CallExpr *CE) const;
+
+  void evalFopen(CheckerContext , const CallExpr *CE) const;
+  void evalTmpfile(CheckerContext , const CallExpr *CE) const;
+  void evalFclose(CheckerContext , const CallExpr *CE) const;
+  void evalFread(CheckerContext , const CallExpr *CE) const;
+  void evalFwrite(CheckerContext , const CallExpr *CE) const;
+  void evalFseek(CheckerContext , const CallExpr *CE) const;
+  void evalFtell(CheckerContext , const CallExpr *CE) const;
+  void evalRewind(CheckerContext , const CallExpr *CE) const;
+  void evalFgetpos(CheckerContext , const CallExpr *CE) const;
+  void evalFsetpos(CheckerContext , const CallExpr *CE) const;
+  void evalClearerr(CheckerContext , const CallExpr *CE) const;
+  void evalFeof(CheckerContext , const CallExpr *CE) const;
+  void evalFerror(CheckerContext , const CallExpr *CE) const;
+  void evalFileno(CheckerContext , const CallExpr *CE) const;
 
   void OpenFileAux(CheckerContext , const CallExpr *CE) const;
 
@@ -115,120 +130,58 @@
   if (!CE)
 return false;
 
-  if (Call.isCalled(CD_fopen)) {
-Fopen(C, CE);
-return true;
-  }
-  if (Call.isCalled(CD_tmpfile)) {
-Tmpfile(C, CE);
-return true;
-  }
-  if (Call.isCalled(CD_fclose)) {
-Fclose(C, CE);
-return true;
-  }
-  if (Call.isCalled(CD_fread)) {
-Fread(C, CE);
-return true;
-  }
-  if (Call.isCalled(CD_fwrite)) {
-Fwrite(C, CE);
-return true;
-  }
-  if (Call.isCalled(CD_fseek)) {
-Fseek(C, CE);
-return true;
-  }
-  if (Call.isCalled(CD_ftell)) {
-Ftell(C, CE);
-return true;
-  }
-  if (Call.isCalled(CD_rewind)) {
-Rewind(C, CE);
-return true;
-  }
-  if 

[PATCH] D67567: [clang-tidy] New check to warn when storing dispatch_once_t in non-static, non-global storage

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D67567#1685077 , @mwyman wrote:

> In D67567#1685036 , @gribozavr wrote:
>
> > Sorry, I reverted it in r373032 because the test fails on Linux: 
> > http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/18323 . 
> > Could you please take a look? Thanks!
>
>
> What's the process for re-landing a change? New Differential, or update this 
> one?


Either works; I think most people update the existing one.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67567



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

[PATCH] D68120: [clangd] Handle type template parameters in findExplicitReferences

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Add tests for non-type template paremeters


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68120

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -706,6 +706,26 @@
"0: targets = {x}\n"
"1: targets = {X::func, X::func}\n"
"2: targets = {t}\n"},
+  // Type template parameters.
+  {R"cpp(
+template 
+void foo() {
+  static_cast<$0^T>(0);
+  $1^T();
+  $2^T t;
+}
+)cpp",
+   "0: targets = {T}\n"
+   "1: targets = {T}\n"
+   "2: targets = {T}\n"},
+  // Non-type template parameters.
+  {R"cpp(
+template 
+void foo() {
+  int x = $0^I;
+}
+)cpp",
+   "0: targets = {I}\n"},
   };
 
   for (const auto  : Cases) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -489,6 +489,11 @@
   ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), 
{L.getDecl()}};
 }
 
+void VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc L) {
+  Ref =
+  ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), 
{L.getDecl()}};
+}
+
 void VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc L) {
   Ref = ReferenceLoc{
   NestedNameSpecifierLoc(), L.getTemplateNameLoc(),


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -706,6 +706,26 @@
"0: targets = {x}\n"
"1: targets = {X::func, X::func}\n"
"2: targets = {t}\n"},
+  // Type template parameters.
+  {R"cpp(
+template 
+void foo() {
+  static_cast<$0^T>(0);
+  $1^T();
+  $2^T t;
+}
+)cpp",
+   "0: targets = {T}\n"
+   "1: targets = {T}\n"
+   "2: targets = {T}\n"},
+  // Non-type template parameters.
+  {R"cpp(
+template 
+void foo() {
+  int x = $0^I;
+}
+)cpp",
+   "0: targets = {I}\n"},
   };
 
   for (const auto  : Cases) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -489,6 +489,11 @@
   ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), {L.getDecl()}};
 }
 
+void VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc L) {
+  Ref =
+  ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), {L.getDecl()}};
+}
+
 void VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc L) {
   Ref = ReferenceLoc{
   NestedNameSpecifierLoc(), L.getTemplateNameLoc(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68120: [clangd] Handle type template parameters in findExplicitReferences

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373067: [clangd] Handle type template parameters in 
findExplicitReferences (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68120?vs=222126=222127#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68120

Files:
  clang-tools-extra/trunk/clangd/FindTarget.cpp
  clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp
@@ -706,6 +706,26 @@
"0: targets = {x}\n"
"1: targets = {X::func, X::func}\n"
"2: targets = {t}\n"},
+  // Type template parameters.
+  {R"cpp(
+template 
+void foo() {
+  static_cast<$0^T>(0);
+  $1^T();
+  $2^T t;
+}
+)cpp",
+   "0: targets = {T}\n"
+   "1: targets = {T}\n"
+   "2: targets = {T}\n"},
+  // Non-type template parameters.
+  {R"cpp(
+template 
+void foo() {
+  int x = $0^I;
+}
+)cpp",
+   "0: targets = {I}\n"},
   };
 
   for (const auto  : Cases) {
Index: clang-tools-extra/trunk/clangd/FindTarget.cpp
===
--- clang-tools-extra/trunk/clangd/FindTarget.cpp
+++ clang-tools-extra/trunk/clangd/FindTarget.cpp
@@ -489,6 +489,11 @@
   ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), 
{L.getDecl()}};
 }
 
+void VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc L) {
+  Ref =
+  ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), 
{L.getDecl()}};
+}
+
 void VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc L) {
   Ref = ReferenceLoc{
   NestedNameSpecifierLoc(), L.getTemplateNameLoc(),


Index: clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp
@@ -706,6 +706,26 @@
"0: targets = {x}\n"
"1: targets = {X::func, X::func}\n"
"2: targets = {t}\n"},
+  // Type template parameters.
+  {R"cpp(
+template 
+void foo() {
+  static_cast<$0^T>(0);
+  $1^T();
+  $2^T t;
+}
+)cpp",
+   "0: targets = {T}\n"
+   "1: targets = {T}\n"
+   "2: targets = {T}\n"},
+  // Non-type template parameters.
+  {R"cpp(
+template 
+void foo() {
+  int x = $0^I;
+}
+)cpp",
+   "0: targets = {I}\n"},
   };
 
   for (const auto  : Cases) {
Index: clang-tools-extra/trunk/clangd/FindTarget.cpp
===
--- clang-tools-extra/trunk/clangd/FindTarget.cpp
+++ clang-tools-extra/trunk/clangd/FindTarget.cpp
@@ -489,6 +489,11 @@
   ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), {L.getDecl()}};
 }
 
+void VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc L) {
+  Ref =
+  ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), {L.getDecl()}};
+}
+
 void VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc L) {
   Ref = ReferenceLoc{
   NestedNameSpecifierLoc(), L.getTemplateNameLoc(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r373068 - Fixed indentation in a ClangTidy test

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Fri Sep 27 03:58:10 2019
New Revision: 373068

URL: http://llvm.org/viewvc/llvm-project?rev=373068=rev
Log:
Fixed indentation in a ClangTidy test

Modified:

clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.mm

Modified: 
clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.mm
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.mm?rev=373068=373067=373068=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.mm
 (original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.mm
 Fri Sep 27 03:58:10 2019
@@ -6,5 +6,5 @@ static NSString* const myConstString = @
 // CHECK-FIXES: static NSString* const kMyConstString = @"hello";
 
 class MyTest {
-static int not_objc_style;
+  static int not_objc_style;
 };


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

[clang-tools-extra] r373067 - [clangd] Handle type template parameters in findExplicitReferences

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Sep 27 03:55:53 2019
New Revision: 373067

URL: http://llvm.org/viewvc/llvm-project?rev=373067=rev
Log:
[clangd] Handle type template parameters in findExplicitReferences

Reviewers: kadircet

Reviewed By: kadircet

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/FindTarget.cpp
clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp

Modified: clang-tools-extra/trunk/clangd/FindTarget.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FindTarget.cpp?rev=373067=373066=373067=diff
==
--- clang-tools-extra/trunk/clangd/FindTarget.cpp (original)
+++ clang-tools-extra/trunk/clangd/FindTarget.cpp Fri Sep 27 03:55:53 2019
@@ -489,6 +489,11 @@ Optional refInTypeLoc(Type
   ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), 
{L.getDecl()}};
 }
 
+void VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc L) {
+  Ref =
+  ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), 
{L.getDecl()}};
+}
+
 void VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc L) {
   Ref = ReferenceLoc{
   NestedNameSpecifierLoc(), L.getTemplateNameLoc(),

Modified: clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp?rev=373067=373066=373067=diff
==
--- clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/FindTargetTests.cpp Fri Sep 27 
03:55:53 2019
@@ -706,6 +706,26 @@ TEST_F(FindExplicitReferencesTest, All)
"0: targets = {x}\n"
"1: targets = {X::func, X::func}\n"
"2: targets = {t}\n"},
+  // Type template parameters.
+  {R"cpp(
+template 
+void foo() {
+  static_cast<$0^T>(0);
+  $1^T();
+  $2^T t;
+}
+)cpp",
+   "0: targets = {T}\n"
+   "1: targets = {T}\n"
+   "2: targets = {T}\n"},
+  // Non-type template parameters.
+  {R"cpp(
+template 
+void foo() {
+  int x = $0^I;
+}
+)cpp",
+   "0: targets = {I}\n"},
   };
 
   for (const auto  : Cases) {


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

[clang-tools-extra] r373066 - Moved -fblocks from an individual test to check_clang_tidy.py

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Fri Sep 27 03:54:28 2019
New Revision: 373066

URL: http://llvm.org/viewvc/llvm-project?rev=373066=rev
Log:
Moved -fblocks from an individual test to check_clang_tidy.py

This way, all tests will benefit from it and will not have to worry
about setting up language options properly.

Modified:
clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
clang-tools-extra/trunk/test/clang-tidy/darwin-dispatch-once-nonstatic.mm

Modified: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py?rev=373066=373065=373066=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py (original)
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py Fri Sep 27 
03:54:28 2019
@@ -69,7 +69,8 @@ def run_test_once(args, extra_args):
 clang_tidy_extra_args.append('-format-style=none')
 
   if extension in ['.m', '.mm']:
-clang_extra_args = ['-fobjc-abi-version=2', '-fobjc-arc'] + 
clang_extra_args
+clang_extra_args = ['-fobjc-abi-version=2', '-fobjc-arc', '-fblocks'] + \
+clang_extra_args
 
   if extension in ['.cpp', '.hpp', '.mm']:
 clang_extra_args.append('-std=' + std)

Modified: 
clang-tools-extra/trunk/test/clang-tidy/darwin-dispatch-once-nonstatic.mm
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/darwin-dispatch-once-nonstatic.mm?rev=373066=373065=373066=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/darwin-dispatch-once-nonstatic.mm 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/darwin-dispatch-once-nonstatic.mm 
Fri Sep 27 03:54:28 2019
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s darwin-dispatch-once-nonstatic %t -- -- -fblocks
+// RUN: %check_clang_tidy %s darwin-dispatch-once-nonstatic %t
 
 typedef int dispatch_once_t;
 extern void dispatch_once(dispatch_once_t *pred, void(^block)(void));


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

[clang-tools-extra] r373065 - [clang-tidy] New check to warn when storing dispatch_once_t in non-static, non-global storage.

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Fri Sep 27 03:49:12 2019
New Revision: 373065

URL: http://llvm.org/viewvc/llvm-project?rev=373065=rev
Log:
[clang-tidy] New check to warn when storing dispatch_once_t in non-static, 
non-global storage.

Summary:
Creates a new darwin ClangTidy module and adds the 
darwin-dispatch-once-nonstatic check that warns about dispatch_once_t variables 
not in static or global storage. This catches a missing static for local 
variables in e.g. singleton initialization behavior, and also warns on storing 
dispatch_once_t values in Objective-C instance variables. C/C++ struct/class 
instances may potentially live in static/global storage, and are ignored for 
this check.

The osx.API static analysis checker can find the non-static storage use of 
dispatch_once_t; I thought it useful to also catch this issue in clang-tidy 
when possible.

This is a re-land of https://reviews.llvm.org/D67567

Reviewers: thakis, gribozavr, stephanemoore

Subscribers: Eugene.Zelenko, mgorny, xazax.hun, jkorous, arphaman, kadircet, 
usaxena95

Tags: #clang-tools-extra, #clang, #llvm

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

Added:
clang-tools-extra/trunk/clang-tidy/darwin/
clang-tools-extra/trunk/clang-tidy/darwin/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/darwin/DarwinTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.cpp
clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/darwin-dispatch-once-nonstatic.rst
clang-tools-extra/trunk/test/clang-tidy/darwin-dispatch-once-nonstatic.mm
Modified:
clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h
clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/CMakeLists.txt?rev=373065=373064=373065=diff
==
--- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt Fri Sep 27 03:49:12 2019
@@ -43,6 +43,7 @@ add_subdirectory(boost)
 add_subdirectory(bugprone)
 add_subdirectory(cert)
 add_subdirectory(cppcoreguidelines)
+add_subdirectory(darwin)
 add_subdirectory(fuchsia)
 add_subdirectory(google)
 add_subdirectory(hicpp)

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h?rev=373065=373064=373065=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h Fri Sep 27 
03:49:12 2019
@@ -50,6 +50,11 @@ extern volatile int CppCoreGuidelinesMod
 static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination =
 CppCoreGuidelinesModuleAnchorSource;
 
+// This anchor is used to force the linker to link the DarwinModule.
+extern volatile int DarwinModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED DarwinModuleAnchorDestination =
+DarwinModuleAnchorSource;
+
 // This anchor is used to force the linker to link the FuchsiaModule.
 extern volatile int FuchsiaModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED FuchsiaModuleAnchorDestination =

Added: clang-tools-extra/trunk/clang-tidy/darwin/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/darwin/CMakeLists.txt?rev=373065=auto
==
--- clang-tools-extra/trunk/clang-tidy/darwin/CMakeLists.txt (added)
+++ clang-tools-extra/trunk/clang-tidy/darwin/CMakeLists.txt Fri Sep 27 
03:49:12 2019
@@ -0,0 +1,15 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyDarwinModule
+  DarwinTidyModule.cpp
+  DispatchOnceNonstaticCheck.cpp
+
+  LINK_LIBS
+  clangAnalysis
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangTidy
+  clangTidyUtils
+  )

Added: clang-tools-extra/trunk/clang-tidy/darwin/DarwinTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/darwin/DarwinTidyModule.cpp?rev=373065=auto
==
--- clang-tools-extra/trunk/clang-tidy/darwin/DarwinTidyModule.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/darwin/DarwinTidyModule.cpp Fri Sep 27 
03:49:12 2019
@@ -0,0 +1,37 @@
+//===--- MiscTidyModule.cpp - clang-tidy 
--===//
+//
+// Part of the LLVM Project, under the 

[PATCH] D68072: Reference qualifiers in member templates causing extra indentation.

[Ilya Biryukov via Phabricator via cfe-commits]

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

The patch breaks some tests, see
http://lab.llvm.org:8011/builders/clang-cmake-x86_64-sde-avx512-linux/builds/27891/steps/ninja%20check%201/logs/FAIL%3A%20Clang-Unit%3A%3AFormatTest.FunctionAnnotations

Happy to re-land when this is fixed.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68072



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

[PATCH] D68124: [clangd] Fix template type aliases in findExplicitReference

[Ilya Biryukov via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/FindTarget.cpp:478
+  explicitReferenceTargets(DynTypedNode::create(L.getType()),
+   DeclRelation::Alias)};
 }

kadircet wrote:
> can you explain why these two are special and get to keep `Alias` decls? 
> Whereas the others dont?
There are two cases to consider for a reference of the form `Templ` in 
type positions:
1. `Templ` is a template type alias
```
template 
Templ = vector;
```
in this case `targetDecl` returns `Templ` with mask `Alias | TemplatePattern`  
and `vector` with mask `Underlying`
since we are interested in the thing explicitly written in the source code, we 
want `Templ` and **not** `vector`.

2. `Templ` is a name of some template type (a class or template template 
parameter)
in this case `targetDecl` returns `Templ` with mask `TemplatePattern` and 
that's what we want to return, since this is what was written in the source 
code.

So in both cases the important thing is to **not** filter-out the result with 
mask `Alias`. This is exactly what this patch does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68124



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

[PATCH] D67907: [clangd] Implement a smart version of HeaderSource switch.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 222112.
hokein added a comment.

format the testcases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67907

Files:
  clang-tools-extra/clangd/HeaderSourceSwitch.cpp
  clang-tools-extra/clangd/HeaderSourceSwitch.h
  clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp

Index: clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
+++ clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
@@ -10,6 +10,7 @@
 
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/MemIndex.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -71,6 +72,174 @@
   EXPECT_FALSE(PathResult.hasValue());
 }
 
+MATCHER_P(DeclNamed, Name, "") {
+  if (const NamedDecl *ND = dyn_cast(arg))
+if (ND->getQualifiedNameAsString() == Name)
+  return true;
+  return false;
+}
+
+TEST(HeaderSourceSwitchTest, GetLocalDecls) {
+  TestTU TU;
+  TU.HeaderCode = R"cpp(
+  void HeaderOnly();
+  )cpp";
+  TU.Code = R"cpp(
+  void MainF1();
+  class Foo {};
+  namespace ns {
+  class Foo {
+void method();
+int field;
+  };
+  } // namespace ns
+
+  // Non-indexable symbols
+  namespace {
+  void Ignore1() {}
+  }
+
+  )cpp";
+
+  auto AST = TU.build();
+  EXPECT_THAT(getIndexableLocalDecls(AST),
+  testing::UnorderedElementsAre(
+  DeclNamed("MainF1"), DeclNamed("Foo"), DeclNamed("ns::Foo"),
+  DeclNamed("ns::Foo::method"), DeclNamed("ns::Foo::field")));
+}
+
+TEST(HeaderSourceSwitchTest, FromHeaderToSource) {
+  // build a proper index, which contains symbols:
+  //   A_Sym1, declared in TestTU.h, defined in a.cpp
+  //   B_Sym[1-2], declared in TestTU.h, defined in b.cpp
+  SymbolSlab::Builder AllSymbols;
+  TestTU Testing;
+  Testing.HeaderFilename = "TestTU.h";
+  Testing.HeaderCode = "void A_Sym1();";
+  Testing.Filename = "a.cpp";
+  Testing.Code = "void A_Sym1() {};";
+  for (auto  : Testing.headerSymbols())
+AllSymbols.insert(Sym);
+
+  Testing.HeaderCode = R"cpp(
+  void B_Sym1();
+  void B_Sym2();
+  )cpp";
+  Testing.Filename = "b.cpp";
+  Testing.Code = R"cpp(
+  void B_Sym1() {}
+  void B_Sym2() {}
+  )cpp";
+  for (auto  : Testing.headerSymbols())
+AllSymbols.insert(Sym);
+  auto Index = MemIndex::build(std::move(AllSymbols).build(), {}, {});
+
+  // Test for swtich from .h header to .cc source
+  struct {
+llvm::StringRef HeaderCode;
+llvm::Optional ExpectedSource;
+  } TestCases[] = {
+  {"// empty, no header found", llvm::None},
+  {R"cpp(
+ // no definition found in the index.
+ void NonDefinition();
+   )cpp",
+   llvm::None},
+  {R"cpp(
+ void A_Sym1();
+   )cpp",
+   testPath("a.cpp")},
+  {R"cpp(
+ // b.cpp wins.
+ void A_Sym1();
+ void B_Sym1();
+ void B_Sym2();
+   )cpp",
+   testPath("b.cpp")},
+  {R"cpp(
+ // a.cpp and b.cpp have same scope, but a.cpp because "a.cpp" < "b.cpp".
+ void A_Sym1();
+ void B_Sym1();
+   )cpp",
+   testPath("a.cpp")},
+  };
+  for (const auto  : TestCases) {
+TestTU TU = TestTU::withCode(Case.HeaderCode);
+TU.Filename = "TestTU.h";
+TU.ExtraArgs.push_back("-xc++-header"); // inform clang this is a header.
+auto HeaderAST = TU.build();
+EXPECT_EQ(Case.ExpectedSource,
+  getCorrespondingHeaderOrSource(testPath(TU.Filename), HeaderAST,
+ Index.get()));
+  }
+}
+
+TEST(HeaderSourceSwitchTest, FromSourceToHeader) {
+  // build a proper index, which contains symbols:
+  //   A_Sym1, declared in a.h, defined in TestTU.cpp
+  //   B_Sym[1-2], declared in b.h, defined in TestTU.cpp
+  TestTU TUForIndex = TestTU::withCode(R"cpp(
+  #include "a.h"
+  #include "b.h"
+
+  void A_Sym1() {}
+
+  void B_Sym1() {}
+  void B_Sym2() {}
+  )cpp");
+  TUForIndex.AdditionalFiles["a.h"] = R"cpp(
+  void A_Sym1();
+  )cpp";
+  TUForIndex.AdditionalFiles["b.h"] = R"cpp(
+  void B_Sym1();
+  void B_Sym2();
+  )cpp";
+  TUForIndex.Filename = "TestTU.cpp";
+  auto Index = TUForIndex.index();
+
+  // Test for switching from .cc source file to .h header.
+  struct {
+llvm::StringRef SourceCode;
+llvm::Optional ExpectedResult;
+  } TestCases[] = {
+  {R"cpp(// empty, no header found)cpp", llvm::None},
+  {R"cpp(
+ // symbol not in index, no header found
+ void Local() {}
+   )cpp",
+   llvm::None},
+
+  {R"cpp(
+ // a.h wins.
+ void A_Sym1() {}
+   )cpp",
+   testPath("a.h")},
+
+  {R"cpp(
+ // b.h wins.
+ void A_Sym1() {}
+ void B_Sym1() {}
+ void B_Sym2() {}
+   )cpp",
+   testPath("b.h")},
+
+  {R"cpp(
+ // a.h and b.h have same scope, 

[PATCH] D67907: [clangd] Implement a smart version of HeaderSource switch.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:86
+  auto AwardTarget = [&](const char *TargetURI) {
+if (auto TargetPath = URI::resolve(TargetURI, OriginalFile)) {
+  if (*TargetPath != OriginalFile) // exclude the original file.

kadircet wrote:
> should we have a cache for these uri resolutions?
> We will most likely have a few URIs that are being resolved over and over 
> again.
> We will most likely have a few URIs that are being resolved over and over 
> again.
This is true, but having a cache seems to be a premature optimization at the 
moment. I think the number for the Decl in the main file is relatively small in 
practice. We could add it when it turns out a performance problem. 



Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:114
+  for (auto It = Candidates.begin(); It != Candidates.end(); ++It) {
+if (It->second > Best->second)
+  Best = It;

kadircet wrote:
> what if there are ties ?
> 
> we don't need to do anything clever yet, but we should at least be 
> deterministic. Currently it depends on stringmap ordering.
> Could you pick the candidate that comes first in the lexical order in such 
> cases?
> and add some test cases?
good point, I thought the StringMap is sorted by the key value, but it uses 
hash value. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67907



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