[PATCH] D48116: [libclang] Allow skipping warnings from all included files

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.
Herald added a project: clang.

>> For filtering in StoredDiagnosticConsumer one needs to pass the new bool 
>> everywhere along where "bool CaptureDiagnostics" is already passed on (to 
>> end up in the StoredDiagnosticConsumer constructor) . Also, 
>> ASTUnit::CaptureDiagnostics is then not enough anymore since the new bool is 
>> also needed in getMainBufferWithPrecompiledPreamble(). One could also (2) 
>> convert  "bool CaptureDiagnostics" to an enum with enumerators like 
>> CaptureNothing, CaptureAll, CaptureAllWithoutNonErrorsFromIncludes to make 
>> this a bit less invasive.
>>  If changing clang's diagnostic interface should be avoided, I tend to go 
>> with (2). Ilya?
> 
> Yeah, LG. The changes in the `ASTUnit` look strictly better than changes in 
> clang - the latter seems to already provide enough to do the filtering.
>  If you avoid changing the `StoredDiagnosticConsumer` (or writing a filtering 
> wrapper for `DiagnosticConsumer`), you'll end up having some diagnostics 
> inside headers generated **after** preamble was built, right?

If there is some #include after the first declaration, possibly. Why is that 
relevant?


Repository:
  rC Clang

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

https://reviews.llvm.org/D48116



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

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

[Nikolai Kosjar via Phabricator via cfe-commits]

nik updated this revision to Diff 187318.
nik added a comment.
Herald added a subscriber: jdoerfert.

OK, filtering happens now in FilterAndStoreDiagnosticConsumer, the former
StoredDiagnosticConsumer.


Repository:
  rC Clang

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

https://reviews.llvm.org/D48116

Files:
  include/clang-c/Index.h
  include/clang/Frontend/ASTUnit.h
  lib/Frontend/ASTUnit.cpp
  test/Index/ignore-warnings-from-headers.cpp
  test/Index/ignore-warnings-from-headers.h
  tools/c-index-test/c-index-test.c
  tools/c-index-test/core_main.cpp
  tools/libclang/CIndex.cpp
  tools/libclang/Indexing.cpp
  unittests/Frontend/ASTUnitTest.cpp
  unittests/Frontend/PCHPreambleTest.cpp

Index: unittests/Frontend/PCHPreambleTest.cpp
===
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -96,8 +96,8 @@
 FileManager *FileMgr = new FileManager(FSOpts, VFS);
 
 std::unique_ptr AST = ASTUnit::LoadFromCompilerInvocation(
-  CI, PCHContainerOpts, Diags, FileMgr, false, false,
-  /*PrecompilePreambleAfterNParses=*/1);
+CI, PCHContainerOpts, Diags, FileMgr, false, CaptureDiagsKind::None,
+/*PrecompilePreambleAfterNParses=*/1);
 return AST;
   }
 
Index: unittests/Frontend/ASTUnitTest.cpp
===
--- unittests/Frontend/ASTUnitTest.cpp
+++ unittests/Frontend/ASTUnitTest.cpp
@@ -51,8 +51,8 @@
 PCHContainerOps = std::make_shared();
 
 return ASTUnit::LoadFromCompilerInvocation(
-CInvok, PCHContainerOps, Diags, FileMgr, false, false, 0, TU_Complete,
-false, false, isVolatile);
+CInvok, PCHContainerOps, Diags, FileMgr, false, CaptureDiagsKind::None,
+0, TU_Complete, false, false, isVolatile);
   }
 };
 
Index: tools/libclang/Indexing.cpp
===
--- tools/libclang/Indexing.cpp
+++ tools/libclang/Indexing.cpp
@@ -443,10 +443,14 @@
   if (CXXIdx->isOptEnabled(CXGlobalOpt_ThreadBackgroundPriorityForIndexing))
 setThreadBackgroundPriority();
 
-  bool CaptureDiagnostics = !Logger::isLoggingEnabled();
+  CaptureDiagsKind CaptureDiagnostics = CaptureDiagsKind::All;
+  if (TU_options & CXTranslationUnit_IgnoreNonErrorsFromIncludedFiles)
+CaptureDiagnostics = CaptureDiagsKind::AllWithoutNonErrorsFromIncludes;
+  if (Logger::isLoggingEnabled())
+CaptureDiagnostics = CaptureDiagsKind::None;
 
   CaptureDiagnosticConsumer *CaptureDiag = nullptr;
-  if (CaptureDiagnostics)
+  if (CaptureDiagnostics != CaptureDiagsKind::None)
 CaptureDiag = new CaptureDiagnosticConsumer();
 
   // Configure the diagnostics.
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3338,7 +3338,7 @@
   ASTUnit::LoadEverything, Diags,
   FileSystemOpts, /*UseDebugInfo=*/false,
   CXXIdx->getOnlyLocalDecls(), None,
-  /*CaptureDiagnostics=*/true,
+  CaptureDiagsKind::All,
   /*AllowPCHWithCompilerErrors=*/true,
   /*UserFilesAreVolatile=*/true);
   *out_TU = MakeCXTranslationUnit(CXXIdx, std::move(AU));
@@ -3411,6 +3411,10 @@
   if (options & CXTranslationUnit_KeepGoing)
 Diags->setSuppressAfterFatalError(false);
 
+  CaptureDiagsKind CaptureDiagnostics = CaptureDiagsKind::All;
+  if (options & CXTranslationUnit_IgnoreNonErrorsFromIncludedFiles)
+CaptureDiagnostics = CaptureDiagsKind::AllWithoutNonErrorsFromIncludes;
+
   // Recover resources if we crash before exiting this function.
   llvm::CrashRecoveryContextCleanupRegistrar >
@@ -3488,7 +3492,7 @@
   Args->data(), Args->data() + Args->size(),
   CXXIdx->getPCHContainerOperations(), Diags,
   CXXIdx->getClangResourcesPath(), CXXIdx->getOnlyLocalDecls(),
-  /*CaptureDiagnostics=*/true, *RemappedFiles.get(),
+  CaptureDiagnostics, *RemappedFiles.get(),
   /*RemappedFilesKeepOriginalName=*/true, PrecompilePreambleAfterNParses,
   TUKind, CacheCodeCompletionResults, IncludeBriefCommentsInCodeCompletion,
   /*AllowPCHWithCompilerErrors=*/true, SkipFunctionBodies, SingleFileParse,
Index: tools/c-index-test/core_main.cpp
===
--- tools/c-index-test/core_main.cpp
+++ tools/c-index-test/core_main.cpp
@@ -264,7 +264,7 @@
   modulePath, *pchRdr, ASTUnit::LoadASTOnly, Diags,
   FileSystemOpts, /*UseDebugInfo=*/false,
   /*OnlyLocalDecls=*/true, None,
-  /*CaptureDiagnostics=*/false,
+  CaptureDiagsKind::None,
   /*AllowPCHWithCompilerErrors=*/true,
   /*UserFilesAreVolatile=*/false);
   if (!AU) {
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -88,6 +88,8 @@
 options |= CXTrans

[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D58345#1401213 , @hokein wrote:

> In D58345#1401040 , @ioeric wrote:
>
> > Looks great! Thanks for doing this.
> >
> > Could you also check in the tool that is used to generate the mapping? We 
> > need a way to update the mapping when cppreference is updated.
>
>
> The tool is not ready yet, I'm still polishing it, but the results are good, 
> I think this patch should not be blocked by the tool.


Generated files aren't really source code, I agree with Eric we should check in 
the tool with this patch.




Comment at: clangd/StdSymbolMap.imp:1
+//===-- StdSymbolMap.imp - ---*- C++ 
-*-===//
+//

Even if we don't use tablegen to produce it, maybe we want to produce similar 
output for flexibility? e.g.
```
SYMBOL(_Exit, std::, "");
```
with multiple entries (or multiple `AMBIGUOUS_SYMBOL` entries) for symbols 
mapped to several headers.



Comment at: clangd/StdSymbolMap.imp:1
+//===-- StdSymbolMap.imp - ---*- C++ 
-*-===//
+//

sammccall wrote:
> Even if we don't use tablegen to produce it, maybe we want to produce similar 
> output for flexibility? e.g.
> ```
> SYMBOL(_Exit, std::, "");
> ```
> with multiple entries (or multiple `AMBIGUOUS_SYMBOL` entries) for symbols 
> mapped to several headers.
is there precedent for `.imp`? I think this should probably be `.inc`



Comment at: clangd/StdSymbolMap.imp:410
+{ "std::gslice_array", { "" } },
+{ "std::hardware_constructive_interference_size", { "" } }, // C++11
+{ "std::hardware_destructive_interference_size", { "" } }, // C++11

I'm not sure if the language comments are useful to human readers, who can just 
look this stuff up. If you think they might be useful to code, we could include 
them in  tablegen-style macros.

In any case, this is a C++17 symbol, so maybe something's wrong with the 
generator.




Comment at: clangd/StdSymbolMap.imp:1248
+{ "std::zetta", { "" } }, // C++11
+{ "std::abs", { "", "", "", "" } },
+{ "std::acos", { "", "", "" } },

Now we come to the ambiguous cases :-)

I think it's actually quite clear what to do here:
 - the `` and `` are equivalent, but `` is only in 
C++17, so we should probably prefer ``
 - the `` version is written as "abs(std::complex)" in cppreference. 
We can tag that one with the string "complex" and prefer it if the symbol's 
signature contains "complex".
 - '` is the same as ``



Comment at: clangd/StdSymbolMap.imp:1256
+{ "std::atanh", { "", "" } }, // C++11
+{ "std::basic_filebuf", { "", "" } },
+{ "std::consume_header", { "", "" } }, // C++11

I can't find a definition in ?



Comment at: clangd/StdSymbolMap.imp:1257
+{ "std::basic_filebuf", { "", "" } },
+{ "std::consume_header", { "", "" } }, // C++11
+{ "std::cos", { "", "", "" } },

I can't find a definition for this in ?



Comment at: clangd/StdSymbolMap.imp:1283
+{ "std::log10", { "", "", "" } },
+{ "std::make_error_code", { "", "" } }, // C++11
+{ "std::make_error_condition", { "", "" } }, // C++11

we're missing the `future` variant I think?



Comment at: clangd/StdSymbolMap.imp:1291
+{ "std::sinh", { "", "", "" } },
+{ "std::size_t", { "", "", "", "", 
"", "" } },
+{ "std::sqrt", { "", "", "" } },

this is interesting: any of the options are valid. IMO the best one here is 
``.
But we should be picking by policy, not by looking at the header where the 
symbol is actually defined. We could do this on either the generator or 
consumer side.



Comment at: clangd/index/CanonicalIncludes.cpp:47
+  // headers (e.g. std::move from  and ), using
+  // qualified name can not disambiguate headers. Instead we should return all
+  // headers and do the disambiguation in clangd side.

Looking at the actual data, I'm not sure this is the right strategy. (and it's 
not clear what "the clangd side" is :-)

The ambiguous cases seem to break down into:
 - cases where we should just pick one, e.g. cstdlib vs cmath for std::abs
 - cases where a function is overloaded for a particular type (e.g. complex), 
where passing more information in here (strawman: "the signature as a string") 
would let us get this correct. Alternatively (and easier), I believe always 
using the "primary" version of this function is going to be correct if not 
*strictly* iwyu, as the type comes from the same header. So In the short term 
I'd suggest just blacklisting the variants.
 - cases like std::move and... maybe one or two more? In the short term always 
returning `` seems good enough. In the long term, passing in the 
signature again would let us disambiguate here.


Repository:
  rCTE Clang Tools Extra

CHANGES

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

[Tolga Mizrak via Phabricator via cfe-commits]

to-miz added a comment.

I understand that adding new options adds costs to the development of 
clang-format, but there was a feature request and there is interest for this 
option.
As for the cost to the development, there already exists PPDIS_AfterHash. All I 
did was make the changes introduced in that patch a bit more general and 
removed the hard coded assumption that there are no indents before a hash at a 
couple of places.
I would argue the cost of development of this new option is roughly the same as 
it is with the existence of PPDIS_AfterHash itself. PPDIS_BeforeHash seems like 
a natural extension of the changes introduced by PPDIS_AfterHash.


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

https://reviews.llvm.org/D52150



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

[PATCH] D58377: Remove extraneous space in MSVC-style diagnostic output

[Hans Wennborg via Phabricator via cfe-commits]

hans created this revision.
hans added a reviewer: thakis.
Herald added a subscriber: jdoerfert.

There was an extra space between the file location and the diagnostic message:

  /tmp/a.c(1,12):  warning: unused parameter 'unused'

the tests didn't catch this due to FileCheck not running in --strict-whitespace 
mode.


https://reviews.llvm.org/D58377

Files:
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/test/Misc/diag-format.c


Index: clang/test/Misc/diag-format.c
===
--- clang/test/Misc/diag-format.c
+++ clang/test/Misc/diag-format.c
@@ -1,30 +1,30 @@
-// RUN: %clang -fsyntax-only  %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
-// RUN: %clang -fsyntax-only -fdiagnostics-format=clang %s 2>&1 | FileCheck %s 
-check-prefix=DEFAULT
-// RUN: %clang -fsyntax-only -fdiagnostics-format=clang -target 
x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang -fsyntax-only  %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=DEFAULT
+// RUN: %clang -fsyntax-only -fdiagnostics-format=clang %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=DEFAULT
+// RUN: %clang -fsyntax-only -fdiagnostics-format=clang -target 
x86_64-pc-win32 %s 2>&1 | FileCheck %s --strict-whitespace -check-prefix=DEFAULT
 //
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300  %s 
2>&1 | FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00  %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1800 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2013
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 
%s 2>&1 | FileCheck %s -check-prefix=MSVC
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1900 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2015
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 -fshow-column %s 2>&1 
| FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1800 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2013
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 
-fshow-column %s 2>&1 | FileCheck %s -check-prefix=MSVC
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1900 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2015
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300  %s 
2>&1 | FileCheck %s --strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00  %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1800 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=MSVC2013
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 
%s 2>&1 | FileCheck %s --strict-whitespace -check-prefix=MSVC
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1900 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=MSVC2015
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 -fshow-column %s 2>&1 
| FileCheck %s --strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1800 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=MSVC2013
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 
-fshow-column %s 2>&1 | FileCheck %s --strict-whitespace -check-prefix=MSVC
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1900 
-t

[PATCH] D58377: Remove extraneous space in MSVC-style diagnostic output

[Hans Wennborg via Phabricator via cfe-commits]

hans updated this revision to Diff 187320.
hans added a comment.

Output a char instead of a one-character string.


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

https://reviews.llvm.org/D58377

Files:
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/test/Misc/diag-format.c


Index: clang/test/Misc/diag-format.c
===
--- clang/test/Misc/diag-format.c
+++ clang/test/Misc/diag-format.c
@@ -1,30 +1,30 @@
-// RUN: %clang -fsyntax-only  %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
-// RUN: %clang -fsyntax-only -fdiagnostics-format=clang %s 2>&1 | FileCheck %s 
-check-prefix=DEFAULT
-// RUN: %clang -fsyntax-only -fdiagnostics-format=clang -target 
x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang -fsyntax-only  %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=DEFAULT
+// RUN: %clang -fsyntax-only -fdiagnostics-format=clang %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=DEFAULT
+// RUN: %clang -fsyntax-only -fdiagnostics-format=clang -target 
x86_64-pc-win32 %s 2>&1 | FileCheck %s --strict-whitespace -check-prefix=DEFAULT
 //
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300  %s 
2>&1 | FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00  %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1800 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2013
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 
%s 2>&1 | FileCheck %s -check-prefix=MSVC
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1900 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2015
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 -fshow-column %s 2>&1 
| FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1800 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2013
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 
-fshow-column %s 2>&1 | FileCheck %s -check-prefix=MSVC
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1900 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2015
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300  %s 
2>&1 | FileCheck %s --strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00  %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1800 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=MSVC2013
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 
%s 2>&1 | FileCheck %s --strict-whitespace -check-prefix=MSVC
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1900 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=MSVC2015
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 -fshow-column %s 2>&1 
| FileCheck %s --strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1800 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=MSVC2013
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 
-fshow-column %s 2>&1 | FileCheck %s --strict-whitespace -check-prefix=MSVC
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1900 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=MSVC2015
 //
-// RUN: %clang -fsyntax-only -f

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: lib/Sema/SemaCast.cpp:2224
+  } else if (IsLValueCast) {
 Kind = CK_LValueBitCast;
   } else if (DestType->isObjCObjectPointerType()) {

Anastasia wrote:
> ebevhan wrote:
> > This might not be applicable to this patch, but just something I noticed.
> > 
> > So `reinterpret_cast` only operates on pointers when dealing with address 
> > spaces. What about something like
> > ```
> > T a;
> > T& a_ref = reinterpret_cast(a);
> > ```
> > `reinterpret_cast` on an lvalue like this is equivalent to 
> > `*reinterpret_cast(&a)`. So for AS code:
> > ```
> > __private T x;
> > __generic T& ref = reinterpret_cast<__generic T&>(x);
> > ```
> > This should work, since `*reinterpret_cast<__generic T*>(&x)` is valid, 
> > correct?
> > 
> > What if we have the reference cast case with a different address space like 
> > this? Doesn't the `IsLValueCast` check need to be first?
> > 
> > What if we have the reference cast case with a different address space like 
> > this? Doesn't the IsLValueCast check need to be first?
> 
> Ok, let me see if I understand you. I changed `__generic` -> `__global` since 
> it's invalid and the error is produced as follows:
>   test.cl:7:21: error: reinterpret_cast from 'int' to '__global int &' is not 
> allowed
>   __global int& ref = reinterpret_cast<__global int&>(x);
> 
> Is this not what we are expecting here?
My last sentence could have been a bit clearer. Yes, for the 
`__private`->`__global` case, it should error. But I was thinking more of the 
case where the AS conversion is valid, like `__private`->`__generic`. Then we 
will do the AS conversion, but we should have done both an AS conversion and an 
`LValueBitCast`, because we need to both convert the 'pointer' and also 
dereference it.


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

https://reviews.llvm.org/D58346



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

[PATCH] D58294: [clangd] Enable indexing of template type parameters

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Yeah you are right, no need to have those in the index. I thought we looked at 
only index for findreferences(which turned out to be wrong, we also traverse 
ast), therefore I had turned this functionality on for dynamic index.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58294



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

[PATCH] D58294: [clangd] Enable indexing of template type parameters

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Turn off indexing of template type parms for dynamic index


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58294

Files:
  clangd/XRefs.cpp
  unittests/clangd/SymbolInfoTests.cpp
  unittests/clangd/XRefsTests.cpp


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -285,11 +285,10 @@
 }
   )cpp",
 
-  /* FIXME: clangIndex doesn't handle template type parameters
   R"cpp(// Template type parameter
-template <[[typename T]]>
+template 
 void foo() { ^T t; }
-  )cpp", */
+  )cpp",
 
   R"cpp(// Namespace
 namespace $decl[[ns]] {
Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -213,14 +213,14 @@
 T^T t;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("TT", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Template parameter reference - type param
   template struct bar {
 int a = N^N;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("NN", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Class member reference - objec
   struct foo {
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -39,7 +39,7 @@
   if (const auto *FD = dyn_cast(D))
 return FD->getDefinition();
   // Only a single declaration is allowed.
-  if (isa(D)) // except cases above
+  if (isa(D) || isa(D)) // except cases above
 return D;
   // Multiple definitions are allowed.
   return nullptr; // except cases above
@@ -243,6 +243,7 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
   IndexOpts.IndexParametersInDeclarations = true;
+  IndexOpts.IndexTemplateParmDecls = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 
@@ -441,6 +442,7 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
   IndexOpts.IndexParametersInDeclarations = true;
+  IndexOpts.IndexTemplateParmDecls = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), RefFinder, IndexOpts);
   return std::move(RefFinder).take();


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -285,11 +285,10 @@
 }
   )cpp",
 
-  /* FIXME: clangIndex doesn't handle template type parameters
   R"cpp(// Template type parameter
-template <[[typename T]]>
+template 
 void foo() { ^T t; }
-  )cpp", */
+  )cpp",
 
   R"cpp(// Namespace
 namespace $decl[[ns]] {
Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -213,14 +213,14 @@
 T^T t;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("TT", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Template parameter reference - type param
   template struct bar {
 int a = N^N;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("NN", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Class member reference - objec
   struct foo {
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -39,7 +39,7 @@
   if (const auto *FD = dyn_cast(D))
 return FD->getDefinition();
   // Only a single declaration is allowed.
-  if (isa(D)) // except cases above
+  if (isa(D) || isa(D)) // except cases above
 return D;
   // Multiple definitions are allowed.
   return nullptr; // except cases above
@@ -243,6 +243,7 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
   IndexOpts.IndexParametersInDeclarations = true;
+  IndexOpts.IndexTemplateParmDecls = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 
@@ -441,6 +442,7 @@
   index::Ind

[PATCH] D58294: [clangd] Enable indexing of template type parameters

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/XRefs.cpp:42
   // Only a single declaration is allowed.
-  if (isa(D)) // except cases above
+  if (isa(D) || isa(D)) // except cases above
 return D;

Should probably also handle `TemplateTemplateParmDecl`?
Could we add tests for template template parameters too?
```
// two most-used constructs with template template parameter references.
template class U>
struct Foo {
  ^U f;
  Foo<^U> x;
};
```


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58294



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

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

This LGTM wrt to layering, i.e. it's nice we don't need to change the code of 
diagnostics.
It's been a while since I've looked at the ASTUnit code, though, would be good 
if someone else took an extra look at it.

Added a few nits too.




Comment at: lib/Frontend/ASTUnit.cpp:682
+  auto &M = D.getSourceManager();
+  return M.isInMainFile(M.getExpansionLoc(D.getLocation()));
+}

`isWrittenInMainFile` might be a better fit: it does not look at presumed 
locations. That would be the expected behavior in the most common case, i.e. 
showing errors in an IDE or a text editor.



Comment at: lib/Frontend/ASTUnit.cpp:694
+  if ((!Info.hasSourceManager() || &Info.getSourceManager() == SourceMgr) &&
+  (StoredDiags || StandaloneDiags)) {
+if (!CaptureNonErrorsFromIncludes

Why do we need this extra check? We checked for StoredDiags and Standalone 
diags in the body of the statement later again.


Repository:
  rC Clang

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

https://reviews.llvm.org/D48116



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

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

If I understand it correctly, this is more of an infrastructure improvement 
than check enhancement. It looks like a nice and clean code. Where do we expect 
to use this new behavior? In the current check or in the upcoming "modernize" 
check?




Comment at: clang-tidy/utils/ExceptionAnalyzer.h:128
+/// throw, if it's unknown or if it won't throw.
+enum State Behaviour : 4;
+

Why 4 bits, not just 2? Can we expect additional states?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57883



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

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

[Roman Lebedev via Phabricator via cfe-commits]

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

Sorry for the reviews, i'm really stalling it seems..

This looks like a straight-forward change, but i'm not sure i'm familiar enough 
with ExceptionAnalyzer to fully properly review this..
As far as i can tell, this is ok. Perhaps @baloghadamsoftware will have other 
remarks.

In D57883#1402023 , 
@baloghadamsoftware wrote:

> If I understand it correctly, this is more of an infrastructure improvement 
> than check enhancement. It looks like a nice and clean code. Where do we 
> expect to use this new behavior? In the current check or in the upcoming 
> "modernize" check?


It's for D57108 , i'we guessed that such 
ternary answer will be required there.




Comment at: clang-tidy/utils/ExceptionAnalyzer.h:218
   llvm::StringSet<> IgnoredExceptions;
-  llvm::DenseMap FunctionCache;
+  std::map FunctionCache;
 };

JonasToth wrote:
> lebedev.ri wrote:
> > Why can't `llvm::DenseMap` continue to be used?
> I would need to add traits for `ExceptionInfo` to work with `DenseMap`. So i 
> went this way :)
> From the docs `DenseMap` shall map small types (like ptr-ptr) but the 
> `ExceptionInfo` has the set of types as `SmallSet`, so not sure if 
> that is actually a good fit.
Good point.



Comment at: clang-tidy/utils/ExceptionAnalyzer.h:26-31
+  enum class State : std::int8_t {
+Throwing,///< The function can definitly throw given an AST.
+NotThrowing, ///< This function can not throw, given an AST.
+Unknown, ///< This can happen for extern functions without available
+ ///< definition.
+  };

Since this is later used in a bit field, it might be better to be explicit 
```
  enum class State : std::int8_t {
Throwing = 0,///< The function can definitly throw given an AST.
NotThrowing = 1, ///< This function can not throw, given an AST.
Unknown = 2, ///< This can happen for extern functions without available
 ///< definition.
  };
```
and indeed, only 2 bits needed.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57883



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

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware accepted this revision.
baloghadamsoftware added a comment.

In D57883#1402049 , @lebedev.ri wrote:

> It's for D57108 , i'we guessed that such 
> ternary answer will be required there.


Oh, I was not aware of that patch.

Anyway, it looks good to me.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57883



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

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaCast.cpp:2224
+  } else if (IsLValueCast) {
 Kind = CK_LValueBitCast;
   } else if (DestType->isObjCObjectPointerType()) {

ebevhan wrote:
> Anastasia wrote:
> > ebevhan wrote:
> > > This might not be applicable to this patch, but just something I noticed.
> > > 
> > > So `reinterpret_cast` only operates on pointers when dealing with address 
> > > spaces. What about something like
> > > ```
> > > T a;
> > > T& a_ref = reinterpret_cast(a);
> > > ```
> > > `reinterpret_cast` on an lvalue like this is equivalent to 
> > > `*reinterpret_cast(&a)`. So for AS code:
> > > ```
> > > __private T x;
> > > __generic T& ref = reinterpret_cast<__generic T&>(x);
> > > ```
> > > This should work, since `*reinterpret_cast<__generic T*>(&x)` is valid, 
> > > correct?
> > > 
> > > What if we have the reference cast case with a different address space 
> > > like this? Doesn't the `IsLValueCast` check need to be first?
> > > 
> > > What if we have the reference cast case with a different address space 
> > > like this? Doesn't the IsLValueCast check need to be first?
> > 
> > Ok, let me see if I understand you. I changed `__generic` -> `__global` 
> > since it's invalid and the error is produced as follows:
> >   test.cl:7:21: error: reinterpret_cast from 'int' to '__global int &' is 
> > not allowed
> >   __global int& ref = reinterpret_cast<__global int&>(x);
> > 
> > Is this not what we are expecting here?
> My last sentence could have been a bit clearer. Yes, for the 
> `__private`->`__global` case, it should error. But I was thinking more of the 
> case where the AS conversion is valid, like `__private`->`__generic`. Then we 
> will do the AS conversion, but we should have done both an AS conversion and 
> an `LValueBitCast`, because we need to both convert the 'pointer' and also 
> dereference it.
Hmm... it seems like here we can only have one cast kind... I guess 
`CK_LValueBitCast` leads to the generation of `bitcast` in the IR? But 
`addrspacecast` can perform both bit reinterpretation and address translation, 
so perhaps it makes sense to only have an address space conversion in this 
case? Unless some other logic is needed elsewhere before CodeGen... I will try 
to construct a test case in plain C++. 


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

https://reviews.llvm.org/D58346



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

[PATCH] D54295: [WIP, RISCV] Add inline asm constraint A for RISC-V

[Lewis Revill via Phabricator via cfe-commits]

lewis-revill updated this revision to Diff 187335.
lewis-revill added a comment.
Herald added subscribers: llvm-commits, jdoerfert.
Herald added a project: LLVM.

Correct test.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54295

Files:
  lib/Basic/Targets/RISCV.cpp
  test/CodeGen/riscv-inline-asm.c


Index: test/CodeGen/riscv-inline-asm.c
===
--- test/CodeGen/riscv-inline-asm.c
+++ test/CodeGen/riscv-inline-asm.c
@@ -26,3 +26,9 @@
 // CHECK: call void asm sideeffect "", "K"(i32 0)
   asm volatile ("" :: "K"(0));
 }
+
+void test_A(int *p) {
+// CHECK-LABEL: define void @test_A(i32* %p)
+// CHECK: call void asm sideeffect "", "*A"(i32* %p)
+  asm volatile("" :: "A"(*p));
+}
Index: lib/Basic/Targets/RISCV.cpp
===
--- lib/Basic/Targets/RISCV.cpp
+++ lib/Basic/Targets/RISCV.cpp
@@ -56,6 +56,10 @@
 // A 5-bit unsigned immediate for CSR access instructions.
 Info.setRequiresImmediate(0, 31);
 return true;
+  case 'A':
+// An address that is held in a general-purpose register.
+Info.setAllowsMemory();
+return true;
   }
 }
 


Index: test/CodeGen/riscv-inline-asm.c
===
--- test/CodeGen/riscv-inline-asm.c
+++ test/CodeGen/riscv-inline-asm.c
@@ -26,3 +26,9 @@
 // CHECK: call void asm sideeffect "", "K"(i32 0)
   asm volatile ("" :: "K"(0));
 }
+
+void test_A(int *p) {
+// CHECK-LABEL: define void @test_A(i32* %p)
+// CHECK: call void asm sideeffect "", "*A"(i32* %p)
+  asm volatile("" :: "A"(*p));
+}
Index: lib/Basic/Targets/RISCV.cpp
===
--- lib/Basic/Targets/RISCV.cpp
+++ lib/Basic/Targets/RISCV.cpp
@@ -56,6 +56,10 @@
 // A 5-bit unsigned immediate for CSR access instructions.
 Info.setRequiresImmediate(0, 31);
 return true;
+  case 'A':
+// An address that is held in a general-purpose register.
+Info.setAllowsMemory();
+return true;
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.
Herald added a project: clang.

Ping?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57450



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

[PATCH] D57896: Variable names rule

[Michael Platings via Phabricator via cfe-commits]

michaelplatings updated this revision to Diff 187339.
michaelplatings added a comment.

Changed recommendation for acronyms from lower case to upper case, as suggested 
by several responses to the RFC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896

Files:
  .clang-tidy
  clang/.clang-tidy
  llvm/.clang-tidy
  llvm/docs/CodingStandards.rst

Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -311,13 +311,13 @@
 
.. code-block:: c++
 
- Object.emitName(nullptr);
+ object.emitName(nullptr);
 
An in-line C-style comment makes the intent obvious:
 
.. code-block:: c++
 
- Object.emitName(/*Prefix=*/nullptr);
+ object.emitName(/*prefix=*/nullptr);
 
 Commenting out large blocks of code is discouraged, but if you really have to do
 this (for documentation purposes or as a suggestion for debug printing), use
@@ -355,8 +355,8 @@
 
 .. code-block:: c++
 
-  /// Sets the xyzzy property to \p Baz.
-  void setXyzzy(bool Baz);
+  /// Sets the xyzzy property to \p baz.
+  void setXyzzy(bool baz);
 
 A documentation comment that uses all Doxygen features in a preferred way:
 
@@ -364,18 +364,18 @@
 
   /// Does foo and bar.
   ///
-  /// Does not do foo the usual way if \p Baz is true.
+  /// Does not do foo the usual way if \p baz is true.
   ///
   /// Typical usage:
   /// \code
-  ///   fooBar(false, "quux", Res);
+  ///   fooBar(false, "quux", res);
   /// \endcode
   ///
-  /// \param Quux kind of foo to do.
+  /// \param quux kind of foo to do.
   /// \param [out] Result filled with bar sequence on foo success.
   ///
   /// \returns true on success.
-  bool fooBar(bool Baz, StringRef Quux, std::vector &Result);
+  bool fooBar(bool baz, StringRef quux, std::vector &result);
 
 Don't duplicate the documentation comment in the header file and in the
 implementation file.  Put the documentation comments for public APIs into the
@@ -603,7 +603,7 @@
 
   foo({a, b, c}, {1, 2, 3});
 
-  llvm::Constant *Mask[] = {
+  llvm::Constant *mask[] = {
   llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 0),
   llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 1),
   llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 2)};
@@ -633,7 +633,7 @@
 
 .. code-block:: c++
 
-  if (V = getValue()) {
+  if (v = getValue()) {
 ...
   }
 
@@ -644,7 +644,7 @@
 
 .. code-block:: c++
 
-  if ((V = getValue())) {
+  if ((v = getValue())) {
 ...
   }
 
@@ -736,7 +736,7 @@
   class Foo;
 
   // Breaks mangling in MSVC.
-  struct Foo { int Data; };
+  struct Foo { int data; };
 
 * As a rule of thumb, ``struct`` should be kept to structures where *all*
   members are declared public.
@@ -746,17 +746,17 @@
   // Foo feels like a class... this is strange.
   struct Foo {
   private:
-int Data;
+int data;
   public:
-Foo() : Data(0) { }
-int getData() const { return Data; }
-void setData(int D) { Data = D; }
+Foo() : data(0) { }
+int getData() const { return data; }
+void setData(int d) { data = d; }
   };
 
   // Bar isn't POD, but it does look like a struct.
   struct Bar {
-int Data;
-Bar() : Data(0) { }
+int data;
+Bar() : data(0) { }
   };
 
 Do not use Braced Initializer Lists to Call a Constructor
@@ -780,7 +780,7 @@
 Foo(std::string filename);
 
 // Construct a Foo by looking up the Nth element of some global data ...
-Foo(int N);
+Foo(int n);
 
 // ...
   };
@@ -789,7 +789,7 @@
   std::fill(foo.begin(), foo.end(), Foo("name"));
 
   // The pair is just being constructed like an aggregate, use braces.
-  bar_map.insert({my_key, my_value});
+  bar_map.insert({myKey, myValue});
 
 If you use a braced initializer list when initializing a variable, use an equals before the open curly brace:
 
@@ -821,15 +821,15 @@
 .. code-block:: c++
 
   // Typically there's no reason to copy.
-  for (const auto &Val : Container) { observe(Val); }
-  for (auto &Val : Container) { Val.change(); }
+  for (const auto &val : container) { observe(val); }
+  for (auto &val : container) { val.change(); }
 
   // Remove the reference if you really want a new copy.
-  for (auto Val : Container) { Val.change(); saveSomewhere(Val); }
+  for (auto val : container) { val.change(); saveSomewhere(val); }
 
   // Copy pointers, but make it clear that they're pointers.
-  for (const auto *Ptr : Container) { observe(*Ptr); }
-  for (auto *Ptr : Container) { Ptr->change(); }
+  for (const auto *ptr : container) { observe(*ptr); }
+  for (auto *ptr : container) { ptr->change(); }
 
 Beware of non-determinism due to ordering of pointers
 ^
@@ -968,9 +968,9 @@
 
 .. code-block:: c++
 
-  Value *doSomething(Instruction *I) {
-if (!I->isTerminator() &&
-

[PATCH] D58179: [OpenCL][PR40707] Allow OpenCL C types in C++ mode

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

Committed in r354121.


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

https://reviews.llvm.org/D58179



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

[PATCH] D58293: [clang][Index] Enable indexing of Template Type Parameters behind a flag

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: lib/Index/IndexingContext.cpp:51
+
 bool IndexingContext::handleDecl(const Decl *D,
  SymbolRoleSet Roles,

ilya-biryukov wrote:
> Do we call `handleDecl` for template parameters now too?
No we don't. I believe having the decl itself is not that useful for a template 
parameter without a reference to it. We only call handlereference.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58293



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

[PATCH] D58293: [clang][Index] Enable indexing of Template Type Parameters behind a flag

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 187341.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Fix handling of TemplateTemplateTypeParams


Repository:
  rC Clang

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

https://reviews.llvm.org/D58293

Files:
  include/clang/Index/IndexingAction.h
  lib/Index/IndexSymbol.cpp
  lib/Index/IndexTypeSourceInfo.cpp
  lib/Index/IndexingContext.cpp
  lib/Index/IndexingContext.h
  unittests/Index/IndexTests.cpp

Index: unittests/Index/IndexTests.cpp
===
--- unittests/Index/IndexTests.cpp
+++ unittests/Index/IndexTests.cpp
@@ -93,6 +93,7 @@
   IndexingOptions Opts;
 };
 
+using testing::AllOf;
 using testing::Contains;
 using testing::Not;
 using testing::UnorderedElementsAre;
@@ -134,6 +135,28 @@
   EXPECT_THAT(Index->Symbols, Not(Contains(QName("bar";
 }
 
+TEST(IndexTest, IndexTypeParmDecls) {
+  std::string Code = R"cpp(
+template  class C> struct Foo {
+  T t = I;
+  C x;
+};
+  )cpp";
+  auto Index = std::make_shared();
+  IndexingOptions Opts;
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, AllOf(Not(Contains(QName("Foo::T"))),
+Not(Contains(QName("Foo::I"))),
+Not(Contains(QName("Foo::C");
+
+  Opts.IndexTemplateParmDecls = true;
+  Index->Symbols.clear();
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols,
+  AllOf(Contains(QName("Foo::T")), Contains(QName("Foo::I")),
+Contains(QName("Foo::C";
+}
+
 } // namespace
 } // namespace index
 } // namespace clang
Index: lib/Index/IndexingContext.h
===
--- lib/Index/IndexingContext.h
+++ lib/Index/IndexingContext.h
@@ -63,6 +63,8 @@
 
   bool shouldIndexParametersInDeclarations() const;
 
+  bool shouldIndexTemplateParmDecls() const;
+
   static bool isTemplateImplicitInstantiation(const Decl *D);
 
   bool handleDecl(const Decl *D, SymbolRoleSet Roles = SymbolRoleSet(),
Index: lib/Index/IndexingContext.cpp
===
--- lib/Index/IndexingContext.cpp
+++ lib/Index/IndexingContext.cpp
@@ -44,6 +44,10 @@
   return IndexOpts.IndexParametersInDeclarations;
 }
 
+bool IndexingContext::shouldIndexTemplateParmDecls() const {
+  return IndexOpts.IndexTemplateParmDecls;
+}
+
 bool IndexingContext::handleDecl(const Decl *D,
  SymbolRoleSet Roles,
  ArrayRef Relations) {
@@ -76,8 +80,11 @@
   if (!shouldIndexFunctionLocalSymbols() && isFunctionLocalSymbol(D))
 return true;
 
-  if (isa(D) || isa(D))
+  if (!shouldIndexTemplateParmDecls() &&
+  (isa(D) || isa(D) ||
+   isa(D))) {
 return true;
+  }
 
   return handleDeclOccurrence(D, Loc, /*IsRef=*/true, Parent, Roles, Relations,
   RefE, RefD, DC);
Index: lib/Index/IndexTypeSourceInfo.cpp
===
--- lib/Index/IndexTypeSourceInfo.cpp
+++ lib/Index/IndexTypeSourceInfo.cpp
@@ -45,6 +45,13 @@
   return false;\
   } while (0)
 
+  bool VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TTPL) {
+SourceLocation Loc = TTPL.getNameLoc();
+TemplateTypeParmDecl *TTPD = TTPL.getDecl();
+return IndexCtx.handleReference(TTPD, Loc, Parent, ParentDC,
+SymbolRoleSet());
+  }
+
   bool VisitTypedefTypeLoc(TypedefTypeLoc TL) {
 SourceLocation Loc = TL.getNameLoc();
 TypedefNameDecl *ND = TL.getTypedefNameDecl();
Index: lib/Index/IndexSymbol.cpp
===
--- lib/Index/IndexSymbol.cpp
+++ lib/Index/IndexSymbol.cpp
@@ -55,9 +55,6 @@
   if (isa(D))
 return true;
 
-  if (isa(D))
-return true;
-
   if (isa(D))
 return true;
 
Index: include/clang/Index/IndexingAction.h
===
--- include/clang/Index/IndexingAction.h
+++ include/clang/Index/IndexingAction.h
@@ -46,6 +46,7 @@
   bool IndexMacrosInPreprocessor = false;
   // Has no effect if IndexFunctionLocals are false.
   bool IndexParametersInDeclarations = false;
+  bool IndexTemplateParmDecls = false;
 };
 
 /// Creates a frontend action that indexes all symbols (macros and AST decls).
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58343: Enablement for AMD znver2 architecture - skeleton patch

[Ganesh Gopalasubramanian via Phabricator via cfe-commits]

GGanesh updated this revision to Diff 187340.
GGanesh added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Addressed the comments from Craig Topper


Repository:
  rC Clang

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

https://reviews.llvm.org/D58343

Files:
  include/clang/Basic/X86Target.def
  lib/Basic/Targets/X86.cpp
  test/CodeGen/target-builtin-noerror.c
  test/Driver/x86-march.c
  test/Frontend/x86-target-cpu.c
  test/Misc/target-invalid-cpu-note.c
  test/Preprocessor/predefined-arch-macros.c

Index: test/Preprocessor/predefined-arch-macros.c
===
--- test/Preprocessor/predefined-arch-macros.c
+++ test/Preprocessor/predefined-arch-macros.c
@@ -2676,6 +2676,100 @@
 // CHECK_ZNVER1_M64: #define __znver1 1
 // CHECK_ZNVER1_M64: #define __znver1__ 1
 
+// RUN: %clang -march=znver2 -m32 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_ZNVER2_M32
+// CHECK_ZNVER2_M32-NOT: #define __3dNOW_A__ 1
+// CHECK_ZNVER2_M32-NOT: #define __3dNOW__ 1
+// CHECK_ZNVER2_M32: #define __ADX__ 1
+// CHECK_ZNVER2_M32: #define __AES__ 1
+// CHECK_ZNVER2_M32: #define __AVX2__ 1
+// CHECK_ZNVER2_M32: #define __AVX__ 1
+// CHECK_ZNVER2_M32: #define __BMI2__ 1
+// CHECK_ZNVER2_M32: #define __BMI__ 1
+// CHECK_ZNVER2_M32: #define __CLFLUSHOPT__ 1
+// CHECK_ZNVER2_M32: #define __CLWB__ 1
+// CHECK_ZNVER2_M32: #define __CLZERO__ 1
+// CHECK_ZNVER2_M32: #define __F16C__ 1
+// CHECK_ZNVER2_M32: #define __FMA__ 1
+// CHECK_ZNVER2_M32: #define __FSGSBASE__ 1
+// CHECK_ZNVER2_M32: #define __LZCNT__ 1
+// CHECK_ZNVER2_M32: #define __MMX__ 1
+// CHECK_ZNVER2_M32: #define __PCLMUL__ 1
+// CHECK_ZNVER2_M32: #define __POPCNT__ 1
+// CHECK_ZNVER2_M32: #define __PRFCHW__ 1
+// CHECK_ZNVER2_M32: #define __RDPID__ 1
+// CHECK_ZNVER2_M32: #define __RDRND__ 1
+// CHECK_ZNVER2_M32: #define __RDSEED__ 1
+// CHECK_ZNVER2_M32: #define __SHA__ 1
+// CHECK_ZNVER2_M32: #define __SSE2_MATH__ 1
+// CHECK_ZNVER2_M32: #define __SSE2__ 1
+// CHECK_ZNVER2_M32: #define __SSE3__ 1
+// CHECK_ZNVER2_M32: #define __SSE4A__ 1
+// CHECK_ZNVER2_M32: #define __SSE4_1__ 1
+// CHECK_ZNVER2_M32: #define __SSE4_2__ 1
+// CHECK_ZNVER2_M32: #define __SSE_MATH__ 1
+// CHECK_ZNVER2_M32: #define __SSE__ 1
+// CHECK_ZNVER2_M32: #define __SSSE3__ 1
+// CHECK_ZNVER2_M32: #define __WBNOINVD__ 1
+// CHECK_ZNVER2_M32: #define __XSAVEC__ 1
+// CHECK_ZNVER2_M32: #define __XSAVEOPT__ 1
+// CHECK_ZNVER2_M32: #define __XSAVES__ 1
+// CHECK_ZNVER2_M32: #define __XSAVE__ 1
+// CHECK_ZNVER2_M32: #define __i386 1
+// CHECK_ZNVER2_M32: #define __i386__ 1
+// CHECK_ZNVER2_M32: #define __tune_znver2__ 1
+// CHECK_ZNVER2_M32: #define __znver2 1
+// CHECK_ZNVER2_M32: #define __znver2__ 1
+
+// RUN: %clang -march=znver2 -m64 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_ZNVER2_M64
+// CHECK_ZNVER2_M64-NOT: #define __3dNOW_A__ 1
+// CHECK_ZNVER2_M64-NOT: #define __3dNOW__ 1
+// CHECK_ZNVER2_M64: #define __ADX__ 1
+// CHECK_ZNVER2_M64: #define __AES__ 1
+// CHECK_ZNVER2_M64: #define __AVX2__ 1
+// CHECK_ZNVER2_M64: #define __AVX__ 1
+// CHECK_ZNVER2_M64: #define __BMI2__ 1
+// CHECK_ZNVER2_M64: #define __BMI__ 1
+// CHECK_ZNVER2_M64: #define __CLFLUSHOPT__ 1
+// CHECK_ZNVER2_M64: #define __CLWB__ 1
+// CHECK_ZNVER2_M64: #define __CLZERO__ 1
+// CHECK_ZNVER2_M64: #define __F16C__ 1
+// CHECK_ZNVER2_M64: #define __FMA__ 1
+// CHECK_ZNVER2_M64: #define __FSGSBASE__ 1
+// CHECK_ZNVER2_M64: #define __LZCNT__ 1
+// CHECK_ZNVER2_M64: #define __MMX__ 1
+// CHECK_ZNVER2_M64: #define __PCLMUL__ 1
+// CHECK_ZNVER2_M64: #define __POPCNT__ 1
+// CHECK_ZNVER2_M64: #define __PRFCHW__ 1
+// CHECK_ZNVER2_M64: #define __RDPID__ 1
+// CHECK_ZNVER2_M64: #define __RDRND__ 1
+// CHECK_ZNVER2_M64: #define __RDSEED__ 1
+// CHECK_ZNVER2_M64: #define __SHA__ 1
+// CHECK_ZNVER2_M64: #define __SSE2_MATH__ 1
+// CHECK_ZNVER2_M64: #define __SSE2__ 1
+// CHECK_ZNVER2_M64: #define __SSE3__ 1
+// CHECK_ZNVER2_M64: #define __SSE4A__ 1
+// CHECK_ZNVER2_M64: #define __SSE4_1__ 1
+// CHECK_ZNVER2_M64: #define __SSE4_2__ 1
+// CHECK_ZNVER2_M64: #define __SSE_MATH__ 1
+// CHECK_ZNVER2_M64: #define __SSE__ 1
+// CHECK_ZNVER2_M64: #define __SSSE3__ 1
+// CHECK_ZNVER2_M64: #define __WBNOINVD__ 1
+// CHECK_ZNVER2_M64: #define __XSAVEC__ 1
+// CHECK_ZNVER2_M64: #define __XSAVEOPT__ 1
+// CHECK_ZNVER2_M64: #define __XSAVES__ 1
+// CHECK_ZNVER2_M64: #define __XSAVE__ 1
+// CHECK_ZNVER2_M64: #define __amd64 1
+// CHECK_ZNVER2_M64: #define __amd64__ 1
+// CHECK_ZNVER2_M64: #define __tune_znver2__ 1
+// CHECK_ZNVER2_M64: #define __x86_64 1
+// CHECK_ZNVER2_M64: #define __x86_64__ 1
+// CHECK_ZNVER2_M64: #define __znver2 1
+// CHECK_ZNVER2_M64: #define __znver2__ 1
+
 // End X86/GCC/Linux tests --
 
 // Begin PPC/GCC/Linux tests 
Index: test/

[PATCH] D58294: [clangd] Enable indexing of template type parameters

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Add handling for template template type parameters


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58294

Files:
  clangd/XRefs.cpp
  unittests/clangd/SymbolInfoTests.cpp
  unittests/clangd/XRefsTests.cpp


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -285,11 +285,15 @@
 }
   )cpp",
 
-  /* FIXME: clangIndex doesn't handle template type parameters
   R"cpp(// Template type parameter
-template <[[typename T]]>
+template 
 void foo() { ^T t; }
-  )cpp", */
+  )cpp",
+
+  R"cpp(// Template template type parameter
+template  class [[T]]>
+void foo() { ^T t; }
+  )cpp",
 
   R"cpp(// Namespace
 namespace $decl[[ns]] {
Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -213,14 +213,14 @@
 T^T t;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("TT", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Template parameter reference - type param
   template struct bar {
 int a = N^N;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("NN", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Class member reference - objec
   struct foo {
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -39,7 +39,8 @@
   if (const auto *FD = dyn_cast(D))
 return FD->getDefinition();
   // Only a single declaration is allowed.
-  if (isa(D)) // except cases above
+  if (isa(D) || isa(D) ||
+  isa(D)) // except cases above
 return D;
   // Multiple definitions are allowed.
   return nullptr; // except cases above
@@ -243,6 +244,7 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
   IndexOpts.IndexParametersInDeclarations = true;
+  IndexOpts.IndexTemplateParmDecls = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 
@@ -441,6 +443,7 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
   IndexOpts.IndexParametersInDeclarations = true;
+  IndexOpts.IndexTemplateParmDecls = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), RefFinder, IndexOpts);
   return std::move(RefFinder).take();


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -285,11 +285,15 @@
 }
   )cpp",
 
-  /* FIXME: clangIndex doesn't handle template type parameters
   R"cpp(// Template type parameter
-template <[[typename T]]>
+template 
 void foo() { ^T t; }
-  )cpp", */
+  )cpp",
+
+  R"cpp(// Template template type parameter
+template  class [[T]]>
+void foo() { ^T t; }
+  )cpp",
 
   R"cpp(// Namespace
 namespace $decl[[ns]] {
Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -213,14 +213,14 @@
 T^T t;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("TT", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Template parameter reference - type param
   template struct bar {
 int a = N^N;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("NN", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Class member reference - objec
   struct foo {
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -39,7 +39,8 @@
   if (const auto *FD = dyn_cast(D))
 return FD->getDefinition();
   // Only a single declaration is allowed.
-  if (isa(D)) // except cases above
+  if (isa(D) || isa(D) ||
+  isa(D)) // except cases above
 return D;
   // Multiple definitions are allowed.
   return nullptr; // except cases above
@@ -243,6 +244,7 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
 

[PATCH] D57896: Variable names rule

[Mikhail Maltsev via Phabricator via cfe-commits]

miyuki added inline comments.



Comment at: llvm/docs/CodingStandards.rst:1065
   case 'J': {
-if (Signed) {
-  Type = Context.getsigjmp_bufType();
-  if (Type.isNull()) {
-Error = ASTContext::GE_Missing_sigjmp_buf;
+if (signed) {
+  type = context.getsigjmp_bufType();

signed is a keyword, it can't be used as a variable name


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Looks sensible to me.

I'm just curious why we want to prevent emission of atomic LLVM instructions at 
this point. Won't LLVM's AtomicExpand perform a similar lowering already? 
Perhaps the goal is to save that pass some work?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57450



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

[PATCH] D58185: [clangd] Handle unresolved scope specifier when fixing includes.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/IncludeFixer.cpp:191
+//~~
+llvm::Optional qualifiedByUnresolved(llvm::StringRef Code,
+  size_t Offset) {

sammccall wrote:
> this isn't wrong per se (conservative, bails out e.g. on unicode)
> But is it much harder to call Lexer::findNextToken twice and expect a raw 
> identifier and ::?
Good idea. Lexer is a better option.



Comment at: clangd/IncludeFixer.cpp:251
+// namespace clang { clangd::X; }
+// In this case, we use the "typo" qualifier as extra scope instead
+// of using the scope assumed by sema.

jkorous wrote:
> Does this work with anonymous namespaces?
I'm not sure... how do you use an anonymous namespace as a scope specifier?



Comment at: clangd/IncludeFixer.cpp:256
+std::string Spelling = (Code.substr(B, E - B) + "::").str();
+vlog("Spelling scope: {0}, SpecifiedNS: {1}", Spelling, SpecifiedNS);
+if (llvm::StringRef(SpecifiedNS).endswith(Spelling))

sammccall wrote:
> I don't think this vlog is useful as-is (quite far down the stack with no 
> context)
> Did you intend to remove it?
Indeed. Thanks for the catch!



Comment at: clangd/IncludeFixer.cpp:271
+  if (IsUnrsolvedSpecifier) {
+// If the unresolved name is a qualifier e.g.
+//  clang::clangd::X

jkorous wrote:
> Is the term `qualifier` applicable here? (Honest question.)
> 
> It seems like C++ grammar uses `specifier` (same as you in 
> `IsUnrsolvedSpecifier `)
> http://www.nongnu.org/hcb/#nested-name-specifier
You've raised a good point. We've been using `specifier` and `qualifier` 
interchangeably in the project. But specifier does seem to be a more 
appropriate name to use.  I've fixed uses in this file. Uses in other files 
probably need a separate cleanup.



Comment at: clangd/IncludeFixer.cpp:322
 UnresolvedName Unresolved;
 Unresolved.Name = Typo.getAsString();
 Unresolved.Loc = Typo.getBeginLoc();

sammccall wrote:
> Following up on our offline discussion :-)
> I think that since `extractSpecifiedScopes` can want to modify the name, we 
> should just expand that function's signature/responsibility to always 
> determine the name.
> 
> So we'd pass the `const DeclarationNameInfo&` to `extractSpecifiedScopes`, 
> and it would return a struct `{optional ResolvedScope; 
> optional UnresolvedScope; string Name}`. 
> Maybe need to call them `CheapUnresolvedName`/`extractUnresolvedNameCheaply` 
> or  similar.
> But I think the code change is small.
Sounds good. Thanks!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58185



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

[PATCH] D58185: [clangd] Handle unresolved scope specifier when fixing includes.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 187343.
ioeric marked 16 inline comments as done.
ioeric added a comment.

- address review comment


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58185

Files:
  clangd/IncludeFixer.cpp
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -20,6 +20,7 @@
 namespace clangd {
 namespace {
 
+using testing::_;
 using testing::ElementsAre;
 using testing::Field;
 using testing::IsEmpty;
@@ -369,6 +370,8 @@
 $insert[[]]namespace ns {
 void foo() {
   $unqualified1[[X]] x;
+  // No fix if the unresolved type is used as specifier. (ns::)X::Nested will be
+  // considered the unresolved type.
   $unqualified2[[X]]::Nested n;
 }
 }
@@ -391,10 +394,7 @@
   AllOf(Diag(Test.range("unqualified1"), "unknown type name 'X'"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X"))),
-  AllOf(Diag(Test.range("unqualified2"),
- "use of undeclared identifier 'X'"),
-WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
-"Add include \"x.h\" for symbol ns::X"))),
+  Diag(Test.range("unqualified2"), "use of undeclared identifier 'X'"),
   AllOf(Diag(Test.range("qualified1"),
  "no type named 'X' in namespace 'ns'"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
@@ -487,6 +487,88 @@
   }
 }
 
+TEST(IncludeFixerTest, UnresolvedNameAsSpecifier) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace ns {
+}
+void g() {  ns::$[[scope]]::X_Y();  }
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  SymbolWithHeader{"ns::scope::X_Y", "unittest:///x.h", "\"x.h\""});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Test.range(), "no member named 'scope' in namespace 'ns'"),
+  WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+  "Add include \"x.h\" for symbol ns::scope::X_Y");
+}
+
+TEST(IncludeFixerTest, UnresolvedSpecifierWithSemaCorrection) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace clang {
+void f() {
+  // "clangd::" will be corrected to "clang::" by Sema.
+  $q1[[clangd]]::$x[[X]] x;
+  $q2[[clangd]]::$ns[[ns]]::Y y;
+}
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  {SymbolWithHeader{"clang::clangd::X", "unittest:///x.h", "\"x.h\""},
+   SymbolWithHeader{"clang::clangd::ns::Y", "unittest:///y.h", "\"y.h\""}});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  AllOf(
+  Diag(Test.range("q1"), "use of undeclared identifier 'clangd'; "
+ "did you mean 'clang'?"),
+  WithFix(_, // change clangd to clang
+  Fix(Test.range("insert"), "#include \"x.h\"\n",
+  "Add include \"x.h\" for symbol clang::clangd::X"))),
+  AllOf(
+  Diag(Test.range("x"), "no type named 'X' in namespace 'clang'"),
+  WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+  "Add include \"x.h\" for symbol clang::clangd::X"))),
+  AllOf(
+  Diag(Test.range("q2"), "use of undeclared identifier 'clangd'; "
+ "did you mean 'clang'?"),
+  WithFix(
+  _, // change clangd to clangd
+  Fix(Test.range("insert"), "#include \"y.h\"\n",
+  "Add include \"y.h\" for symbol clang::clangd::ns::Y"))),
+  AllOf(Diag(Test.range("ns"),
+ "no member named 'ns' in namespace 'clang'"),
+WithFix(Fix(
+Test.range("insert"), "#include \"y.h\"\n",
+"Add include \"y.h\" for symbol clang::clangd::ns::Y");
+}
+
+TEST(IncludeFixerTest, SpecifiedScopeIsNamespaceAlias) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace a {}
+namespace b = a;
+namespace c {
+  b::$[[X]] x;
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  SymbolWithHeader{"a::X", "unittest:///x.h", "\"x.h\""});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Test.range(), "no type named 'X' in namespace 'a'"),
+  WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+  "Add include \"x.h\" for symbol a::X");
+}
+
 } // na

[PATCH] D57896: Variable names rule

[Michael Platings via Phabricator via cfe-commits]

michaelplatings updated this revision to Diff 187344.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896

Files:
  .clang-tidy
  clang/.clang-tidy
  llvm/.clang-tidy
  llvm/docs/CodingStandards.rst

Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -311,13 +311,13 @@
 
.. code-block:: c++
 
- Object.emitName(nullptr);
+ object.emitName(nullptr);
 
An in-line C-style comment makes the intent obvious:
 
.. code-block:: c++
 
- Object.emitName(/*Prefix=*/nullptr);
+ object.emitName(/*prefix=*/nullptr);
 
 Commenting out large blocks of code is discouraged, but if you really have to do
 this (for documentation purposes or as a suggestion for debug printing), use
@@ -355,8 +355,8 @@
 
 .. code-block:: c++
 
-  /// Sets the xyzzy property to \p Baz.
-  void setXyzzy(bool Baz);
+  /// Sets the xyzzy property to \p baz.
+  void setXyzzy(bool baz);
 
 A documentation comment that uses all Doxygen features in a preferred way:
 
@@ -364,18 +364,18 @@
 
   /// Does foo and bar.
   ///
-  /// Does not do foo the usual way if \p Baz is true.
+  /// Does not do foo the usual way if \p baz is true.
   ///
   /// Typical usage:
   /// \code
-  ///   fooBar(false, "quux", Res);
+  ///   fooBar(false, "quux", res);
   /// \endcode
   ///
-  /// \param Quux kind of foo to do.
+  /// \param quux kind of foo to do.
   /// \param [out] Result filled with bar sequence on foo success.
   ///
   /// \returns true on success.
-  bool fooBar(bool Baz, StringRef Quux, std::vector &Result);
+  bool fooBar(bool baz, StringRef quux, std::vector &result);
 
 Don't duplicate the documentation comment in the header file and in the
 implementation file.  Put the documentation comments for public APIs into the
@@ -603,7 +603,7 @@
 
   foo({a, b, c}, {1, 2, 3});
 
-  llvm::Constant *Mask[] = {
+  llvm::Constant *mask[] = {
   llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 0),
   llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 1),
   llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 2)};
@@ -633,7 +633,7 @@
 
 .. code-block:: c++
 
-  if (V = getValue()) {
+  if (v = getValue()) {
 ...
   }
 
@@ -644,7 +644,7 @@
 
 .. code-block:: c++
 
-  if ((V = getValue())) {
+  if ((v = getValue())) {
 ...
   }
 
@@ -736,7 +736,7 @@
   class Foo;
 
   // Breaks mangling in MSVC.
-  struct Foo { int Data; };
+  struct Foo { int data; };
 
 * As a rule of thumb, ``struct`` should be kept to structures where *all*
   members are declared public.
@@ -746,17 +746,17 @@
   // Foo feels like a class... this is strange.
   struct Foo {
   private:
-int Data;
+int data;
   public:
-Foo() : Data(0) { }
-int getData() const { return Data; }
-void setData(int D) { Data = D; }
+Foo() : data(0) { }
+int getData() const { return data; }
+void setData(int d) { data = d; }
   };
 
   // Bar isn't POD, but it does look like a struct.
   struct Bar {
-int Data;
-Bar() : Data(0) { }
+int data;
+Bar() : data(0) { }
   };
 
 Do not use Braced Initializer Lists to Call a Constructor
@@ -780,7 +780,7 @@
 Foo(std::string filename);
 
 // Construct a Foo by looking up the Nth element of some global data ...
-Foo(int N);
+Foo(int n);
 
 // ...
   };
@@ -789,7 +789,7 @@
   std::fill(foo.begin(), foo.end(), Foo("name"));
 
   // The pair is just being constructed like an aggregate, use braces.
-  bar_map.insert({my_key, my_value});
+  bar_map.insert({myKey, myValue});
 
 If you use a braced initializer list when initializing a variable, use an equals before the open curly brace:
 
@@ -821,15 +821,15 @@
 .. code-block:: c++
 
   // Typically there's no reason to copy.
-  for (const auto &Val : Container) { observe(Val); }
-  for (auto &Val : Container) { Val.change(); }
+  for (const auto &val : container) { observe(val); }
+  for (auto &val : container) { val.change(); }
 
   // Remove the reference if you really want a new copy.
-  for (auto Val : Container) { Val.change(); saveSomewhere(Val); }
+  for (auto val : container) { val.change(); saveSomewhere(val); }
 
   // Copy pointers, but make it clear that they're pointers.
-  for (const auto *Ptr : Container) { observe(*Ptr); }
-  for (auto *Ptr : Container) { Ptr->change(); }
+  for (const auto *ptr : container) { observe(*ptr); }
+  for (auto *ptr : container) { ptr->change(); }
 
 Beware of non-determinism due to ordering of pointers
 ^
@@ -968,9 +968,9 @@
 
 .. code-block:: c++
 
-  Value *doSomething(Instruction *I) {
-if (!I->isTerminator() &&
-I->hasOneUse() && doOtherThing(I)) {
+  Value *doSomething(Instruction *instruction) {
+if (!instruction->isTerminator() &&
+instruction-

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

In D57450#1402153 , @rogfer01 wrote:

> Looks sensible to me.
>
> I'm just curious why we want to prevent emission of atomic LLVM instructions 
> at this point. Won't LLVM's AtomicExpand perform a similar lowering already? 
> Perhaps the goal is to save that pass some work?


There was some discussion about whether the frontend should lower to libcalls 
or not here https://bugs.llvm.org/show_bug.cgi?id=31620 and it seems the 
decided path was that it's better for the frontend to lower, even if 
AtomicExpandPass does have some support for introducing these libcalls. 
Additionally, setting the max atomic inline width should mean that 
`__atomic_is_lock_free` is evaluated correctly.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57450



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

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[James Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

Also worth noting that the frontend needs to know the max width otherwise 
`warn_atomic_op_misaligned` will be triggered with the message `large atomic 
operation may incur significant performance penalty` (name is misleading since 
the `%select{large|misaligned}0` means it's triggered for both large and 
misaligned atomic ops, and it's also in the `atomic-alignment` group). This is 
exactly what I did in my tree (other than the tests) and it seemed to work 
correctly for RV64A compiling FreeBSD.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57450



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

[PATCH] D58294: [clangd] Enable indexing of template type parameters

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58294



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

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:823
+  (*H)->contents.value =
+  llvm::formatv("```C++\n{0}\n```", (*H)->contents.value);
+}

The contents of the hover is not strictly C++ code, so we shouldn't put the 
whole block into the C++ code.
Could we return the plain text results for now? The hover implementation should 
probably create markdown (or some other form of formatted text) on its own, but 
that's out of scope of this patch.



Comment at: clangd/Protocol.h:378
+  /// textDocument.hover.contentFormat.
+  llvm::Optional HoverMarkupKinds;
 };

Could we replace this with a single boolean to simplify the code?
```
bool HoverSupportsMarkdown = false;
```

Would also allow to get remove `MarkupKindBitset`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55250



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

[PATCH] D57896: Variable names rule

[Chris Lattner via Phabricator via cfe-commits]

lattner added a comment.

> Changed recommendation for acronyms from lower case to upper case, as 
> suggested by several responses to the RFC.

I haven't been following the discussion closely - why is this the preferred 
direction?  I don't think that things like "Basicblock *bb" or "MachineInstr 
*mi" will be confusing, and going towards a consistently leading lower case 
letter seems simple and preferable.

-Chris


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

[PATCH] D58306: [AArch64] Change size suffix for FP16FML intrinsics.

[Sjoerd Meijer via Phabricator via cfe-commits]

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

LGTM

The ACLE has been updated and a new version with change included will be 
released soon.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58306



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

[PATCH] D57896: Variable names rule

[Michael Platings via Phabricator via cfe-commits]

michaelplatings added a comment.

In D57896#1402194 , @lattner wrote:

> > Changed recommendation for acronyms from lower case to upper case, as 
> > suggested by several responses to the RFC.
>
> I haven't been following the discussion closely - why is this the preferred 
> direction?  I don't think that things like "Basicblock *bb" or "MachineInstr 
> *mi" will be confusing, and going towards a consistently leading lower case 
> letter seems simple and preferable.


Maybe I misunderstood you 
(http://lists.llvm.org/pipermail/llvm-dev/2019-February/130223.html):

>> Maybe there should be an exception that variable names that start with an 
>> acronym still should start with an upper case letter?
>  That would also be fine with me - it could push such a debate down the road, 
> and is definitely important for a transition period anyway.

Also Chandler 
(http://lists.llvm.org/pipermail/llvm-dev/2019-February/130313.html):

> FWIW, I suspect separating the transition of our acronyms from the transition 
> of identifiers with non-acronym words may be an effective way to chip away at 
> the transition cost... Definitely an area that people who really care about 
> this should look at carefully.

And Sanjoy (kind of) 
(http://lists.llvm.org/pipermail/llvm-dev/2019-February/130304.html):

> maybe a gradual transition plan could be to allow these upper case acronyms 
> for specific classes?

I agree that lower case acronyms would ultimately be more consistent, but given 
where we are it seems more achievable to only change the rule for non-acronyms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

[clang-tools-extra] r354330 - [clangd] Handle unresolved scope specifier when fixing includes.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Tue Feb 19 06:32:22 2019
New Revision: 354330

URL: http://llvm.org/viewvc/llvm-project?rev=354330&view=rev
Log:
[clangd] Handle unresolved scope specifier when fixing includes.

Summary:
In the following examples, "clangd" is unresolved, and the fixer will try to fix
include for `clang::clangd`; however, clang::clangd::X is usually intended. So
when handling a qualifier that is unresolved, we change the unresolved name and
scopes so that the fixer will fix "clang::clangd::X" in the following example.
```
  namespace clang {
clangd::X
~~
  }
  // or
  clang::clangd::X
 ~~
```

Reviewers: sammccall

Reviewed By: sammccall

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/IncludeFixer.cpp
clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp

Modified: clang-tools-extra/trunk/clangd/IncludeFixer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/IncludeFixer.cpp?rev=354330&r1=354329&r2=354330&view=diff
==
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp Tue Feb 19 06:32:22 2019
@@ -19,6 +19,10 @@
 #include "clang/AST/Type.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticSema.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Scope.h"
@@ -28,6 +32,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -172,6 +177,121 @@ std::vector IncludeFixer::fixesForS
   }
   return Fixes;
 }
+
+// Returns the identifiers qualified by an unresolved name. \p Loc is the
+// start location of the unresolved name. For the example below, this returns
+// "::X::Y" that is qualified by unresolved name "clangd":
+// clang::clangd::X::Y
+//~
+llvm::Optional qualifiedByUnresolved(const SourceManager &SM,
+  SourceLocation Loc,
+  const LangOptions &LangOpts) 
{
+  std::string Result;
+
+  SourceLocation NextLoc = Loc;
+  while (auto CCTok = Lexer::findNextToken(NextLoc, SM, LangOpts)) {
+if (!CCTok->is(tok::coloncolon))
+  break;
+auto IDTok = Lexer::findNextToken(CCTok->getLocation(), SM, LangOpts);
+if (!IDTok || !IDTok->is(tok::raw_identifier))
+  break;
+Result.append(("::" + IDTok->getRawIdentifier()).str());
+NextLoc = IDTok->getLocation();
+  }
+  if (Result.empty())
+return llvm::None;
+  return Result;
+}
+
+// An unresolved name and its scope information that can be extracted cheaply.
+struct CheapUnresolvedName {
+  std::string Name;
+  // This is the part of what was typed that was resolved, and it's in its
+  // resolved form not its typed form (think `namespace clang { clangd::x }` 
-->
+  // `clang::clangd::`).
+  llvm::Optional ResolvedScope;
+
+  // Unresolved part of the scope. When the unresolved name is a specifier, we
+  // use the name that comes after it as the alternative name to resolve and 
use
+  // the specifier as the extra scope in the accessible scopes.
+  llvm::Optional UnresolvedScope;
+};
+
+// Extracts unresolved name and scope information around \p Unresolved.
+// FIXME: try to merge this with the scope-wrangling code in CodeComplete.
+llvm::Optional extractUnresolvedNameCheaply(
+const SourceManager &SM, const DeclarationNameInfo &Unresolved,
+CXXScopeSpec *SS, const LangOptions &LangOpts, bool UnresolvedIsSpecifier) 
{
+  bool Invalid = false;
+  llvm::StringRef Code = SM.getBufferData(
+  SM.getDecomposedLoc(Unresolved.getBeginLoc()).first, &Invalid);
+  if (Invalid)
+return llvm::None;
+  CheapUnresolvedName Result;
+  Result.Name = Unresolved.getAsString();
+  if (SS && SS->isNotEmpty()) { // "::" or "ns::"
+if (auto *Nested = SS->getScopeRep()) {
+  if (Nested->getKind() == NestedNameSpecifier::Global)
+Result.ResolvedScope = "";
+  else if (const auto *NS = Nested->getAsNamespace()) {
+auto SpecifiedNS = printNamespaceScope(*NS);
+
+// Check the specifier spelled in the source.
+// If the resolved scope doesn't end with the spelled scope. The
+// resolved scope can come from a sema typo correction. For example,
+// sema assumes that "clangd::" is a typo of "clang::" and uses
+// "clang::" as the specified scope in:
+// namespace clang { clangd::X; }
+// In this case

[PATCH] D58185: [clangd] Handle unresolved scope specifier when fixing includes.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 187359.
ioeric added a comment.

- minor cleanup


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58185

Files:
  clangd/IncludeFixer.cpp
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -20,6 +20,7 @@
 namespace clangd {
 namespace {
 
+using testing::_;
 using testing::ElementsAre;
 using testing::Field;
 using testing::IsEmpty;
@@ -369,6 +370,8 @@
 $insert[[]]namespace ns {
 void foo() {
   $unqualified1[[X]] x;
+  // No fix if the unresolved type is used as specifier. (ns::)X::Nested will be
+  // considered the unresolved type.
   $unqualified2[[X]]::Nested n;
 }
 }
@@ -391,10 +394,7 @@
   AllOf(Diag(Test.range("unqualified1"), "unknown type name 'X'"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
 "Add include \"x.h\" for symbol ns::X"))),
-  AllOf(Diag(Test.range("unqualified2"),
- "use of undeclared identifier 'X'"),
-WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
-"Add include \"x.h\" for symbol ns::X"))),
+  Diag(Test.range("unqualified2"), "use of undeclared identifier 'X'"),
   AllOf(Diag(Test.range("qualified1"),
  "no type named 'X' in namespace 'ns'"),
 WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
@@ -487,6 +487,88 @@
   }
 }
 
+TEST(IncludeFixerTest, UnresolvedNameAsSpecifier) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace ns {
+}
+void g() {  ns::$[[scope]]::X_Y();  }
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  SymbolWithHeader{"ns::scope::X_Y", "unittest:///x.h", "\"x.h\""});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Test.range(), "no member named 'scope' in namespace 'ns'"),
+  WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+  "Add include \"x.h\" for symbol ns::scope::X_Y");
+}
+
+TEST(IncludeFixerTest, UnresolvedSpecifierWithSemaCorrection) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace clang {
+void f() {
+  // "clangd::" will be corrected to "clang::" by Sema.
+  $q1[[clangd]]::$x[[X]] x;
+  $q2[[clangd]]::$ns[[ns]]::Y y;
+}
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  {SymbolWithHeader{"clang::clangd::X", "unittest:///x.h", "\"x.h\""},
+   SymbolWithHeader{"clang::clangd::ns::Y", "unittest:///y.h", "\"y.h\""}});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  AllOf(
+  Diag(Test.range("q1"), "use of undeclared identifier 'clangd'; "
+ "did you mean 'clang'?"),
+  WithFix(_, // change clangd to clang
+  Fix(Test.range("insert"), "#include \"x.h\"\n",
+  "Add include \"x.h\" for symbol clang::clangd::X"))),
+  AllOf(
+  Diag(Test.range("x"), "no type named 'X' in namespace 'clang'"),
+  WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+  "Add include \"x.h\" for symbol clang::clangd::X"))),
+  AllOf(
+  Diag(Test.range("q2"), "use of undeclared identifier 'clangd'; "
+ "did you mean 'clang'?"),
+  WithFix(
+  _, // change clangd to clangd
+  Fix(Test.range("insert"), "#include \"y.h\"\n",
+  "Add include \"y.h\" for symbol clang::clangd::ns::Y"))),
+  AllOf(Diag(Test.range("ns"),
+ "no member named 'ns' in namespace 'clang'"),
+WithFix(Fix(
+Test.range("insert"), "#include \"y.h\"\n",
+"Add include \"y.h\" for symbol clang::clangd::ns::Y");
+}
+
+TEST(IncludeFixerTest, SpecifiedScopeIsNamespaceAlias) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace a {}
+namespace b = a;
+namespace c {
+  b::$[[X]] x;
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  SymbolWithHeader{"a::X", "unittest:///x.h", "\"x.h\""});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Test.range(), "no type named 'X' in namespace 'a'"),
+  WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+  "Add include \"x.h\" for symbol a::X");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clan

[PATCH] D58185: [clangd] Handle unresolved scope specifier when fixing includes.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354330: [clangd] Handle unresolved scope specifier when 
fixing includes. (authored by ioeric, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58185

Files:
  clang-tools-extra/trunk/clangd/IncludeFixer.cpp
  clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp

Index: clang-tools-extra/trunk/clangd/IncludeFixer.cpp
===
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp
+++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp
@@ -19,6 +19,10 @@
 #include "clang/AST/Type.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticSema.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Scope.h"
@@ -28,6 +32,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -172,6 +177,121 @@
   }
   return Fixes;
 }
+
+// Returns the identifiers qualified by an unresolved name. \p Loc is the
+// start location of the unresolved name. For the example below, this returns
+// "::X::Y" that is qualified by unresolved name "clangd":
+// clang::clangd::X::Y
+//~
+llvm::Optional qualifiedByUnresolved(const SourceManager &SM,
+  SourceLocation Loc,
+  const LangOptions &LangOpts) {
+  std::string Result;
+
+  SourceLocation NextLoc = Loc;
+  while (auto CCTok = Lexer::findNextToken(NextLoc, SM, LangOpts)) {
+if (!CCTok->is(tok::coloncolon))
+  break;
+auto IDTok = Lexer::findNextToken(CCTok->getLocation(), SM, LangOpts);
+if (!IDTok || !IDTok->is(tok::raw_identifier))
+  break;
+Result.append(("::" + IDTok->getRawIdentifier()).str());
+NextLoc = IDTok->getLocation();
+  }
+  if (Result.empty())
+return llvm::None;
+  return Result;
+}
+
+// An unresolved name and its scope information that can be extracted cheaply.
+struct CheapUnresolvedName {
+  std::string Name;
+  // This is the part of what was typed that was resolved, and it's in its
+  // resolved form not its typed form (think `namespace clang { clangd::x }` -->
+  // `clang::clangd::`).
+  llvm::Optional ResolvedScope;
+
+  // Unresolved part of the scope. When the unresolved name is a specifier, we
+  // use the name that comes after it as the alternative name to resolve and use
+  // the specifier as the extra scope in the accessible scopes.
+  llvm::Optional UnresolvedScope;
+};
+
+// Extracts unresolved name and scope information around \p Unresolved.
+// FIXME: try to merge this with the scope-wrangling code in CodeComplete.
+llvm::Optional extractUnresolvedNameCheaply(
+const SourceManager &SM, const DeclarationNameInfo &Unresolved,
+CXXScopeSpec *SS, const LangOptions &LangOpts, bool UnresolvedIsSpecifier) {
+  bool Invalid = false;
+  llvm::StringRef Code = SM.getBufferData(
+  SM.getDecomposedLoc(Unresolved.getBeginLoc()).first, &Invalid);
+  if (Invalid)
+return llvm::None;
+  CheapUnresolvedName Result;
+  Result.Name = Unresolved.getAsString();
+  if (SS && SS->isNotEmpty()) { // "::" or "ns::"
+if (auto *Nested = SS->getScopeRep()) {
+  if (Nested->getKind() == NestedNameSpecifier::Global)
+Result.ResolvedScope = "";
+  else if (const auto *NS = Nested->getAsNamespace()) {
+auto SpecifiedNS = printNamespaceScope(*NS);
+
+// Check the specifier spelled in the source.
+// If the resolved scope doesn't end with the spelled scope. The
+// resolved scope can come from a sema typo correction. For example,
+// sema assumes that "clangd::" is a typo of "clang::" and uses
+// "clang::" as the specified scope in:
+// namespace clang { clangd::X; }
+// In this case, we use the "typo" specifier as extra scope instead
+// of using the scope assumed by sema.
+auto B = SM.getFileOffset(SS->getBeginLoc());
+auto E = SM.getFileOffset(SS->getEndLoc());
+std::string Spelling = (Code.substr(B, E - B) + "::").str();
+if (llvm::StringRef(SpecifiedNS).endswith(Spelling))
+  Result.ResolvedScope = SpecifiedNS;
+else
+  Result.UnresolvedScope = Spelling;
+  } else if (const auto *ANS = Nested->getAsNamespaceAlias()) {
+Result.ResolvedScope = printNamespaceScope(*ANS->getNamespace());
+  } else {
+// We don't fix symbols in scopes that are not top-level 

[PATCH] D58320: [Darwin] Introduce a new flag, -flink-builtins-rt that forces linking of the builtins library.

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

The implementation changes in the Darwin toolchain look fine to me, although 
with respect to the command line option I think Petr Hosek's message on cfe-dev 
is interesting:

> GCC implements -nolibc which could be used to achieve the same effect when 
> combined with -nostartfiles (and -nostdlib++ when compiling C++). I'd prefer 
> that approach not only because it improves compatibility with with GCC, but 
> also because it matches existing flag scheme which is subtractive rather than 
> additive (i.e. -nodefaultlibs, -nostdlib, -nostdlib++, -nostartfiles). Clang 
> already defines this flag but the only toolchain that currently supports it 
> is DragonFly.

Looking at https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html (quoted here 
for convenience)

> -nostartfiles
>  Do not use the standard system startup files when linking. The standard 
> system libraries are used normally, unless -nostdlib, -nolibc, or 
> -nodefaultlibs is used.
> -nolibc
>  Do not use the C library or system libraries tightly coupled with it when 
> linking. Still link with the startup files, libgcc or toolchain provided 
> language support libraries such as libgnat, libgfortran or libstdc++ unless 
> options preventing their inclusion are used as well. This typically removes 
> -lc from the link command line, as well as system libraries that normally go 
> with it and become meaningless when absence of a C library is assumed, for 
> example -lpthread or -lm in some configurations. This is intended for 
> bare-board targets when there is indeed no C library available.

It does seem like these options accomplish what -flink_builtins_rt do with the 
added advantage of being more portable with gcc. If they don't work for you it 
will be worth double checking with Petr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58320



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

Re: r354075 - [clang][FileManager] fillRealPathName even if we aren't opening the file

[Nico Weber via cfe-commits]

I didn't realize it just copied a string. I just saw a call to
"fillRealPathName" and the "RealPath" bit sounded slow. From clicking
around in http://llvm-cs.pcc.me.uk/tools/clang/lib/Basic/FileManager.cpp#361
it looks like it's not really computing a realpath (for symlinks). It still
does quite a bit of string processing if InterndFileName isn't absolute,
but that's probably fine. It's still the kind of thing I'd carefully
measure since getFile(openFile=false) was performance-sensitive in a
certain case iirc (I think when using pch files?)

On Mon, Feb 18, 2019 at 5:26 PM Jan Korous  wrote:

> Hi Nico,
>
> I didn't think it necessary as the change doesn't introduce any
> interaction with filesystem - it's just copying a string.
>
> Do you mean it causes a performance regression?
>
> Thanks.
>
> Jan
>
> On Feb 15, 2019, at 6:15 AM, Nico Weber  wrote:
>
> Did you do any performance testing to check if this slows down clang?
>
> On Thu, Feb 14, 2019 at 6:02 PM Jan Korous via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: jkorous
>> Date: Thu Feb 14 15:02:35 2019
>> New Revision: 354075
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=354075&view=rev
>> Log:
>> [clang][FileManager] fillRealPathName even if we aren't opening the file
>>
>> The pathname wasn't previously filled when the getFile() method was
>> called with openFile = false.
>> We are caching FileEntry-s in ParsedAST::Includes in clangd and this
>> caused the problem.
>>
>> This fixes an internal test failure in clangd -
>> ClangdTests.GoToInclude.All
>>
>> rdar://47536127
>>
>> Differential Revision: https://reviews.llvm.org/D58213
>>
>> Modified:
>> cfe/trunk/lib/Basic/FileManager.cpp
>> cfe/trunk/unittests/Basic/FileManagerTest.cpp
>>
>> Modified: cfe/trunk/lib/Basic/FileManager.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=354075&r1=354074&r2=354075&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Basic/FileManager.cpp (original)
>> +++ cfe/trunk/lib/Basic/FileManager.cpp Thu Feb 14 15:02:35 2019
>> @@ -267,6 +267,9 @@ const FileEntry *FileManager::getFile(St
>>if (UFE.File) {
>>  if (auto PathName = UFE.File->getName())
>>fillRealPathName(&UFE, *PathName);
>> +  } else if (!openFile) {
>> +// We should still fill the path even if we aren't opening the file.
>> +fillRealPathName(&UFE, InterndFileName);
>>}
>>return &UFE;
>>  }
>>
>> Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=354075&r1=354074&r2=354075&view=diff
>>
>> ==
>> --- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original)
>> +++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Thu Feb 14 15:02:35 2019
>> @@ -346,4 +346,18 @@ TEST_F(FileManagerTest, getVirtualFileFi
>>EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
>>  }
>>
>> +TEST_F(FileManagerTest, getFileDontOpenRealPath) {
>> +  auto statCache = llvm::make_unique();
>> +  statCache->InjectDirectory("/tmp/abc", 42);
>> +  SmallString<64> Path("/tmp/abc/foo.cpp");
>> +  statCache->InjectFile(Path.str().str().c_str(), 43);
>> +  manager.setStatCache(std::move(statCache));
>> +
>> +  const FileEntry *file = manager.getFile(Path, /*openFile=*/false);
>> +
>> +  ASSERT_TRUE(file != nullptr);
>> +
>> +  ASSERT_EQ(file->tryGetRealPathName(), Path);
>> +}
>> +
>>  } // anonymous namespace
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58377: Remove extraneous space in MSVC-style diagnostic output

[Nico Weber via Phabricator via cfe-commits]

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

wht


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

https://reviews.llvm.org/D58377



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

[PATCH] D58293: [clang][Index] Enable indexing of Template Type Parameters behind a flag

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: include/clang/Index/IndexingAction.h:49
   bool IndexParametersInDeclarations = false;
+  bool IndexTemplateParmDecls = false;
 };

NIT: maybe rename to `IndexTemplateParameters`? The `ParmDecl` is a somewhat 
weird name that does not add much clarity.



Comment at: lib/Index/IndexingContext.cpp:51
+
 bool IndexingContext::handleDecl(const Decl *D,
  SymbolRoleSet Roles,

kadircet wrote:
> ilya-biryukov wrote:
> > Do we call `handleDecl` for template parameters now too?
> No we don't. I believe having the decl itself is not that useful for a 
> template parameter without a reference to it. We only call handlereference.
I'm probably call both just for the sake of symmetry.
How does `IndexParametersInDeclarations` behaves? When set to true will it call 
both `handleDecl` and `handleReference`?
I'd argue we should do the same for template parameters.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58293



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

[PATCH] D57896: Variable names rule

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

In D57896#1402194 , @lattner wrote:

> > Changed recommendation for acronyms from lower case to upper case, as 
> > suggested by several responses to the RFC.
>
> I haven't been following the discussion closely - why is this the preferred 
> direction?  I don't think that things like "Basicblock *bb" or "MachineInstr 
> *mi" will be confusing, and going towards a consistently leading lower case 
> letter seems simple and preferable.
>
> -Chris


I don’t think we should use this review as evidence of consensus.  For example, 
I’m going to be against any change that doesn’t bring us closer to LLDB’s style 
of `lower_case` simply on the grounds that a move which brings us farther away 
from global consistency is strictly worse than one which brings us closer, 
despite ones personal aesthetic preferences.

And so far, I don’t really see that addressed here (or in the thread)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

[PATCH] D57896: Variable names rule

[Zachary Turner via Phabricator via cfe-commits]

zturner requested changes to this revision.
zturner added a comment.
This revision now requires changes to proceed.

Since someone already accepted this, I suppose I should mark require changes to 
formalize my dissent


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

r354337 - [OpenCL] Change type of block pointer for OpenCL

[Alexey Bader via cfe-commits]

Author: bader
Date: Tue Feb 19 07:19:06 2019
New Revision: 354337

URL: http://llvm.org/viewvc/llvm-project?rev=354337&view=rev
Log:
[OpenCL] Change type of block pointer for OpenCL

Summary:

For some reason OpenCL blocks in LLVM IR are represented as function pointers.
These pointers do not point to any real function and never get called. Actually
they point to some structure, which in turn contains pointer to the real block
invoke function.
This patch changes represntation of OpenCL blocks in LLVM IR from function
pointers to pointers to `%struct.__block_literal_generic`.
Such representation allows to avoid unnecessary bitcasts and simplifies
further processing (e.g. translation to SPIR-V ) of the module for targets
which do not support function pointers.

Patch by: Alexey Sotkin.

Reviewers: Anastasia, yaxunl, svenvh

Reviewed By: Anastasia

Subscribers: alexbatashev, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
cfe/trunk/test/CodeGenOpenCL/blocks.cl
cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Modified: cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenTypes.cpp?rev=354337&r1=354336&r2=354337&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenTypes.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenTypes.cpp Tue Feb 19 07:19:06 2019
@@ -635,7 +635,9 @@ llvm::Type *CodeGenTypes::ConvertType(Qu
 
   case Type::BlockPointer: {
 const QualType FTy = cast(Ty)->getPointeeType();
-llvm::Type *PointeeType = ConvertTypeForMem(FTy);
+llvm::Type *PointeeType = CGM.getLangOpts().OpenCL
+  ? CGM.getGenericBlockLiteralType()
+  : ConvertTypeForMem(FTy);
 unsigned AS = Context.getTargetAddressSpace(FTy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;

Modified: cfe/trunk/test/CodeGenOpenCL/blocks.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/blocks.cl?rev=354337&r1=354336&r2=354337&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/blocks.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/blocks.cl Tue Feb 19 07:19:06 2019
@@ -35,11 +35,10 @@ void foo(){
   // SPIR: %[[block_captured:.*]] = getelementptr inbounds <{ i32, i32, i8 
addrspace(4)*, i32 }>, <{ i32, i32, i8 addrspace(4)*, i32 }>* %[[block]], i32 
0, i32 3
   // SPIR: %[[i_value:.*]] = load i32, i32* %i
   // SPIR: store i32 %[[i_value]], i32* %[[block_captured]],
-  // SPIR: %[[blk_ptr:.*]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32 }>* 
%[[block]] to i32 ()*
-  // SPIR: %[[blk_gen_ptr:.*]] = addrspacecast i32 ()* %[[blk_ptr]] to i32 () 
addrspace(4)*
-  // SPIR: store i32 () addrspace(4)* %[[blk_gen_ptr]], i32 () addrspace(4)** 
%[[block_B:.*]],
-  // SPIR: %[[blk_gen_ptr:.*]] = load i32 () addrspace(4)*, i32 () 
addrspace(4)** %[[block_B]]
-  // SPIR: %[[block_literal:.*]] = bitcast i32 () addrspace(4)* 
%[[blk_gen_ptr]] to %struct.__opencl_block_literal_generic addrspace(4)*
+  // SPIR: %[[blk_ptr:.*]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32 }>* 
%[[block]] to %struct.__opencl_block_literal_generic*
+  // SPIR: %[[blk_gen_ptr:.*]] = addrspacecast 
%struct.__opencl_block_literal_generic* %[[blk_ptr]] to 
%struct.__opencl_block_literal_generic addrspace(4)*
+  // SPIR: store %struct.__opencl_block_literal_generic addrspace(4)* 
%[[blk_gen_ptr]], %struct.__opencl_block_literal_generic addrspace(4)** 
%[[block_B:.*]],
+  // SPIR: %[[block_literal:.*]] = load %struct.__opencl_block_literal_generic 
addrspace(4)*, %struct.__opencl_block_literal_generic addrspace(4)** 
%[[block_B]]
   // SPIR: %[[invoke_addr:.*]] = getelementptr inbounds 
%struct.__opencl_block_literal_generic, %struct.__opencl_block_literal_generic 
addrspace(4)* %[[block_literal]], i32 0, i32 2
   // SPIR: %[[blk_gen_ptr:.*]] = bitcast 
%struct.__opencl_block_literal_generic addrspace(4)* %[[block_literal]] to i8 
addrspace(4)*
   // SPIR: %[[invoke_func_ptr:.*]] = load i8 addrspace(4)*, i8 addrspace(4)* 
addrspace(4)* %[[invoke_addr]]
@@ -50,11 +49,10 @@ void foo(){
   // AMDGCN: %[[block_captured:.*]] = getelementptr inbounds <{ i32, i32, i8*, 
i32 }>, <{ i32, i32, i8*, i32 }> addrspace(5)* %[[block]], i32 0, i32 3
   // AMDGCN: %[[i_value:.*]] = load i32, i32 addrspace(5)* %i
   // AMDGCN: store i32 %[[i_value]], i32 addrspace(5)* %[[block_captured]],
-  // AMDGCN: %[[blk_ptr:.*]] = bitcast <{ i32, i32, i8*, i32 }> addrspace(5)* 
%[[block]] to i32 () addrspace(5)*
-  // AMDGCN: %[[blk_gen_ptr:.*]] = addrspacecast i32 () addrspace(5)* 
%[[blk_ptr]] to i32 ()*
-  // AMDGCN: store i32 ()* %[[blk_gen_ptr]], i32 ()* addrspace(5)* 
%[[block_B:.*]],
-  // AMDGCN: %[[blk_gen_ptr:.*]] = load i32 ()*, i32 ()* addrspace(5)* 
%[[block_B]]
-  // AMDGCN: %[[block_

[PATCH] D58277: [OpenCL] Change type of block pointer for OpenCL

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354337: [OpenCL] Change type of block pointer for OpenCL 
(authored by bader, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58277?vs=186988&id=187363#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58277

Files:
  cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
  cfe/trunk/test/CodeGenOpenCL/blocks.cl
  cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Index: cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
@@ -635,7 +635,9 @@
 
   case Type::BlockPointer: {
 const QualType FTy = cast(Ty)->getPointeeType();
-llvm::Type *PointeeType = ConvertTypeForMem(FTy);
+llvm::Type *PointeeType = CGM.getLangOpts().OpenCL
+  ? CGM.getGenericBlockLiteralType()
+  : ConvertTypeForMem(FTy);
 unsigned AS = Context.getTargetAddressSpace(FTy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
Index: cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
===
--- cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
+++ cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
@@ -11,7 +11,7 @@
 
 // For a block global variable, first emit the block literal as a global variable, then emit the block variable itself.
 // COMMON: [[BL_GLOBAL:@__block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*, i8 addrspace(3)*)* [[INV_G:@[^ ]+]] to i8*) to i8 addrspace(4)*) }
-// COMMON: @block_G =  addrspace(1) constant void (i8 addrspace(3)*) addrspace(4)* addrspacecast (void (i8 addrspace(3)*) addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BL_GLOBAL]] to void (i8 addrspace(3)*) addrspace(1)*) to void (i8 addrspace(3)*) addrspace(4)*)
+// COMMON: @block_G = addrspace(1) constant %struct.__opencl_block_literal_generic addrspace(4)* addrspacecast (%struct.__opencl_block_literal_generic addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BL_GLOBAL]] to %struct.__opencl_block_literal_generic addrspace(1)*) to %struct.__opencl_block_literal_generic addrspace(4)*)
 
 // For anonymous blocks without captures, emit block literals as global variable.
 // COMMON: [[BLG1:@__block_literal_global[^ ]*]] = internal addrspace(1) constant { i32, i32, i8 addrspace(4)* } { i32 {{[0-9]+}}, i32 {{[0-9]+}}, i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*, i8 addrspace(3)*)* {{@[^ ]+}} to i8*) to i8 addrspace(4)*) }
@@ -77,9 +77,9 @@
   // COMMON: [[DEF_Q:%[0-9]+]] = load %opencl.queue_t{{.*}}*, %opencl.queue_t{{.*}}** %default_queue
   // COMMON: [[FLAGS:%[0-9]+]] = load i32, i32* %flags
   // COMMON: store i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*)* [[INVL1:@__device_side_enqueue_block_invoke[^ ]*]] to i8*) to i8 addrspace(4)*), i8 addrspace(4)** %block.invoke
-  // B32: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32 addrspace(1)*, i32, i32 addrspace(1)* }>* %block to void ()*
-  // B64: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32 addrspace(1)*, i32 addrspace(1)*, i32 }>* %block to void ()*
-  // COMMON: [[BL_I8:%[0-9]+]] = addrspacecast void ()* [[BL]] to i8 addrspace(4)*
+  // B32: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32 addrspace(1)*, i32, i32 addrspace(1)* }>* %block to %struct.__opencl_block_literal_generic*
+  // B64: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32 addrspace(1)*, i32 addrspace(1)*, i32 }>* %block to %struct.__opencl_block_literal_generic*
+  // COMMON: [[BL_I8:%[0-9]+]] = addrspacecast %struct.__opencl_block_literal_generic* [[BL]] to i8 addrspace(4)*
   // COMMON-LABEL: call i32 @__enqueue_kernel_basic(
   // COMMON-SAME: %opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* byval [[NDR]]{{([0-9]+)?}},
   // COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVLK1:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
@@ -95,8 +95,8 @@
   // COMMON: [[WAIT_EVNT:%[0-9]+]] = addrspacecast %opencl.clk_event_t{{.*}}** %event_wait_list to %opencl.clk_event_t{{.*}}* addrspace(4)*
   // COMMON: [[EVNT:%[0-9]+]] = addrspacecast %opencl.clk_event_t{{.*}}** %clk_event to %opencl.clk_event_t{{.*}}* addrspace(4)*
   // COMMON: store i8 addrspace(4)* addrspacecast (i8* bitcast (void (i8 addrspace(4)*)* [[INVL2:@__device_side_enqueue_block_invoke[^ ]*]] to i8*) to i8 addrspace(4)*), i8 addrspace(4)** %block.invoke
-  // COMMON: [[BL:%[0-9]+]] = bitcast <{ i32, i32, i8 addrspace(4)*, i32

[PATCH] D58341: [clangd] Index UsingDecls

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Add tests for code completion


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58341

Files:
  unittests/clangd/SymbolCollectorTests.cpp
  unittests/clangd/SymbolInfoTests.cpp

Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -167,7 +167,8 @@
 )cpp",
   {CreateExpectedSymbolDetails("foo", "", "c:@F@foo#"),
CreateExpectedSymbolDetails("foo", "", "c:@F@foo#b#"),
-   CreateExpectedSymbolDetails("foo", "", "c:@F@foo#I#")}},
+   CreateExpectedSymbolDetails("foo", "", "c:@F@foo#I#"),
+   CreateExpectedSymbolDetails("foo", "bar::", "c:@N@bar@UD@foo")}},
   {
   R"cpp( // Multiple symbols returned - implicit conversion
   struct foo {};
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -7,8 +7,13 @@
 //===--===//
 
 #include "Annotations.h"
+#include "ClangdServer.h"
+#include "CodeComplete.h"
+#include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/Index.h"
+#include "index/MemIndex.h"
 #include "index/SymbolCollector.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
@@ -50,6 +55,7 @@
 MATCHER_P(Snippet, S, "") {
   return (arg.Name + arg.CompletionSnippetSuffix).str() == S;
 }
+MATCHER_P(CompletionQName, Name, "") { return (arg.Scope + arg.Name) == Name; }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
 MATCHER_P(DeclURI, P, "") {
   return StringRef(arg.CanonicalDeclaration.FileURI) == P;
@@ -325,9 +331,6 @@
 // Namespace alias
 namespace baz = bar;
 
-// FIXME: using declaration is not supported as the IndexAction will ignore
-// implicit declarations (the implicit using shadow declaration) by default,
-// and there is no way to customize this behavior at the moment.
 using bar::v2;
 } // namespace foo
   )";
@@ -354,6 +357,7 @@
AllOf(QName("foo::int32_t"), ForCodeCompletion(true)),
AllOf(QName("foo::v1"), ForCodeCompletion(true)),
AllOf(QName("foo::bar::v2"), ForCodeCompletion(true)),
+   AllOf(QName("foo::v2"), ForCodeCompletion(true)),
AllOf(QName("foo::baz"), ForCodeCompletion(true))}));
 }
 
@@ -1118,6 +1122,48 @@
   AllOf(QName("Public"), Not(ImplementationDetail();
 }
 
+TEST_F(SymbolCollectorTest, UsingDecl) {
+  auto completions = [](const Annotations &Test, SymbolSlab SS) {
+class IgnoreDiagnostics : public DiagnosticsConsumer {
+  void onDiagnosticsReady(PathRef File,
+  std::vector Diagnostics) override {}
+};
+
+MockFSProvider FS;
+MockCompilationDatabase CDB;
+IgnoreDiagnostics DiagConsumer;
+ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+std::unique_ptr OverrideIndex =
+MemIndex::build(std::move(SS), {});
+clangd::CodeCompleteOptions Opts;
+Opts.Index = OverrideIndex.get();
+
+auto File = testPath("foo.cpp");
+runAddDocument(Server, File, Test.code());
+auto CompletionList =
+llvm::cantFail(runCodeComplete(Server, File, Test.point(), Opts));
+return CompletionList;
+  };
+
+  const Annotations Header(R"(
+  void foo();
+  namespace std {
+using ::foo;
+  }
+  void bar() {
+(void)std::^foo;
+  })");
+  runSymbolCollector(Header.code(), /**/ "");
+  EXPECT_THAT(Symbols, Contains(QName("std::foo")));
+
+  const auto Results = completions(Header, std::move(Symbols));
+  EXPECT_THAT(
+  Results.Completions,
+  Contains(AllOf(
+  ReturnType(/*Currently using decls does not export target info*/ ""),
+  CompletionQName("std::foo";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58341: [clangd] Index UsingDecls

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM from my side, with a few NITs.
But let's wait for an LGTM from Haojian too, to make sure his concerns are 
addressed.




Comment at: unittests/clangd/SymbolCollectorTests.cpp:1126
+TEST_F(SymbolCollectorTest, UsingDecl) {
+  auto completions = [](const Annotations &Test, SymbolSlab SS) {
+class IgnoreDiagnostics : public DiagnosticsConsumer {

That's definitely too much setup for such a simple test.
I thought it's possible to wire up a real index in the completion tests, but it 
seems that's not the case. So let's not bother to run an actual completion 
here, ignore my previous comment about adding a test.




Comment at: unittests/clangd/SymbolCollectorTests.cpp:1163
+  Contains(AllOf(
+  ReturnType(/*Currently using decls does not export target info*/ ""),
+  CompletionQName("std::foo";

a typo NIT: s/does not/do not
also, maybe use "type info" or "signature" instead of "target info"?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58341



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

[PATCH] D58293: [clang][Index] Enable indexing of Template Type Parameters behind a flag

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 187370.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Change options name
- Also index decls


Repository:
  rC Clang

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

https://reviews.llvm.org/D58293

Files:
  include/clang/Index/IndexingAction.h
  lib/Index/IndexDecl.cpp
  lib/Index/IndexSymbol.cpp
  lib/Index/IndexTypeSourceInfo.cpp
  lib/Index/IndexingContext.cpp
  lib/Index/IndexingContext.h
  unittests/Index/IndexTests.cpp

Index: unittests/Index/IndexTests.cpp
===
--- unittests/Index/IndexTests.cpp
+++ unittests/Index/IndexTests.cpp
@@ -93,6 +93,7 @@
   IndexingOptions Opts;
 };
 
+using testing::AllOf;
 using testing::Contains;
 using testing::Not;
 using testing::UnorderedElementsAre;
@@ -134,6 +135,29 @@
   EXPECT_THAT(Index->Symbols, Not(Contains(QName("bar";
 }
 
+TEST(IndexTest, IndexTypeParmDecls) {
+  std::string Code = R"cpp(
+template  class C, typename NoRef>
+struct Foo {
+  T t = I;
+  C x;
+};
+  )cpp";
+  auto Index = std::make_shared();
+  IndexingOptions Opts;
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, AllOf(Not(Contains(QName("Foo::T"))),
+Not(Contains(QName("Foo::I"))),
+Not(Contains(QName("Foo::C");
+
+  Opts.IndexTemplateParameters = true;
+  Index->Symbols.clear();
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols,
+  AllOf(Contains(QName("Foo::T")), Contains(QName("Foo::I")),
+Contains(QName("Foo::C")), Contains(QName("Foo::NoRef";
+}
+
 } // namespace
 } // namespace index
 } // namespace clang
Index: lib/Index/IndexingContext.h
===
--- lib/Index/IndexingContext.h
+++ lib/Index/IndexingContext.h
@@ -63,6 +63,8 @@
 
   bool shouldIndexParametersInDeclarations() const;
 
+  bool shouldIndexTemplateParameters() const;
+
   static bool isTemplateImplicitInstantiation(const Decl *D);
 
   bool handleDecl(const Decl *D, SymbolRoleSet Roles = SymbolRoleSet(),
Index: lib/Index/IndexingContext.cpp
===
--- lib/Index/IndexingContext.cpp
+++ lib/Index/IndexingContext.cpp
@@ -44,6 +44,10 @@
   return IndexOpts.IndexParametersInDeclarations;
 }
 
+bool IndexingContext::shouldIndexTemplateParameters() const {
+  return IndexOpts.IndexTemplateParameters;
+}
+
 bool IndexingContext::handleDecl(const Decl *D,
  SymbolRoleSet Roles,
  ArrayRef Relations) {
@@ -76,8 +80,11 @@
   if (!shouldIndexFunctionLocalSymbols() && isFunctionLocalSymbol(D))
 return true;
 
-  if (isa(D) || isa(D))
+  if (!shouldIndexTemplateParameters() &&
+  (isa(D) || isa(D) ||
+   isa(D))) {
 return true;
+  }
 
   return handleDeclOccurrence(D, Loc, /*IsRef=*/true, Parent, Roles, Relations,
   RefE, RefD, DC);
Index: lib/Index/IndexTypeSourceInfo.cpp
===
--- lib/Index/IndexTypeSourceInfo.cpp
+++ lib/Index/IndexTypeSourceInfo.cpp
@@ -45,6 +45,13 @@
   return false;\
   } while (0)
 
+  bool VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TTPL) {
+SourceLocation Loc = TTPL.getNameLoc();
+TemplateTypeParmDecl *TTPD = TTPL.getDecl();
+return IndexCtx.handleReference(TTPD, Loc, Parent, ParentDC,
+SymbolRoleSet());
+  }
+
   bool VisitTypedefTypeLoc(TypedefTypeLoc TL) {
 SourceLocation Loc = TL.getNameLoc();
 TypedefNameDecl *ND = TL.getTypedefNameDecl();
Index: lib/Index/IndexSymbol.cpp
===
--- lib/Index/IndexSymbol.cpp
+++ lib/Index/IndexSymbol.cpp
@@ -55,9 +55,6 @@
   if (isa(D))
 return true;
 
-  if (isa(D))
-return true;
-
   if (isa(D))
 return true;
 
Index: lib/Index/IndexDecl.cpp
===
--- lib/Index/IndexDecl.cpp
+++ lib/Index/IndexDecl.cpp
@@ -675,13 +675,19 @@
 if (const auto *TTP = dyn_cast(TP)) {
   if (TTP->hasDefaultArgument())
 IndexCtx.indexTypeSourceInfo(TTP->getDefaultArgumentInfo(), Parent);
+  if(IndexCtx.shouldIndexTemplateParameters())
+IndexCtx.handleDecl(TTP);
 } else if (const auto *NTTP = dyn_cast(TP)) {
   if (NTTP->hasDefaultArgument())
 IndexCtx.indexBody(NTTP->getDefaultArgument(), Parent);
+  if (IndexCtx.shouldIndexTemplateParameters())
+IndexCtx.handleDecl(NTTP);
 } else if (const auto *TTPD = dyn_cast(TP)) {
   if (TTPD->hasDefaultArgument())
 handl

[PATCH] D58294: [clangd] Enable indexing of template type parameters

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 187371.
kadircet marked an inline comment as done.
kadircet added a comment.

- Option's name has changed


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58294

Files:
  clangd/XRefs.cpp
  unittests/clangd/SymbolInfoTests.cpp
  unittests/clangd/XRefsTests.cpp


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -285,11 +285,15 @@
 }
   )cpp",
 
-  /* FIXME: clangIndex doesn't handle template type parameters
   R"cpp(// Template type parameter
-template <[[typename T]]>
+template 
 void foo() { ^T t; }
-  )cpp", */
+  )cpp",
+
+  R"cpp(// Template template type parameter
+template  class [[T]]>
+void foo() { ^T t; }
+  )cpp",
 
   R"cpp(// Namespace
 namespace $decl[[ns]] {
Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -213,14 +213,14 @@
 T^T t;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("TT", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Template parameter reference - type param
   template struct bar {
 int a = N^N;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("NN", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Class member reference - objec
   struct foo {
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -39,7 +39,8 @@
   if (const auto *FD = dyn_cast(D))
 return FD->getDefinition();
   // Only a single declaration is allowed.
-  if (isa(D)) // except cases above
+  if (isa(D) || isa(D) ||
+  isa(D)) // except cases above
 return D;
   // Multiple definitions are allowed.
   return nullptr; // except cases above
@@ -243,6 +244,7 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
   IndexOpts.IndexParametersInDeclarations = true;
+  IndexOpts.IndexTemplateParameters = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 
@@ -441,6 +443,7 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
   IndexOpts.IndexParametersInDeclarations = true;
+  IndexOpts.IndexTemplateParameters = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), RefFinder, IndexOpts);
   return std::move(RefFinder).take();


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -285,11 +285,15 @@
 }
   )cpp",
 
-  /* FIXME: clangIndex doesn't handle template type parameters
   R"cpp(// Template type parameter
-template <[[typename T]]>
+template 
 void foo() { ^T t; }
-  )cpp", */
+  )cpp",
+
+  R"cpp(// Template template type parameter
+template  class [[T]]>
+void foo() { ^T t; }
+  )cpp",
 
   R"cpp(// Namespace
 namespace $decl[[ns]] {
Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -213,14 +213,14 @@
 T^T t;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("TT", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Template parameter reference - type param
   template struct bar {
 int a = N^N;
   };
 )cpp",
-  {/* not implemented */}},
+  {CreateExpectedSymbolDetails("NN", "bar::", "c:TestTU.cpp@65")}},
   {
   R"cpp( // Class member reference - objec
   struct foo {
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -39,7 +39,8 @@
   if (const auto *FD = dyn_cast(D))
 return FD->getDefinition();
   // Only a single declaration is allowed.
-  if (isa(D)) // except cases above
+  if (isa(D) || isa(D) ||
+  isa(D)) // except cases above
 return D;
   // Multiple definitions are allowed.
   return nullptr; // except cases above
@@ -243,6 +244,7 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunc

[PATCH] D58293: [clang][Index] Enable indexing of Template Type Parameters behind a flag

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: lib/Index/IndexingContext.cpp:51
+
 bool IndexingContext::handleDecl(const Decl *D,
  SymbolRoleSet Roles,

ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > Do we call `handleDecl` for template parameters now too?
> > No we don't. I believe having the decl itself is not that useful for a 
> > template parameter without a reference to it. We only call handlereference.
> I'm probably call both just for the sake of symmetry.
> How does `IndexParametersInDeclarations` behaves? When set to true will it 
> call both `handleDecl` and `handleReference`?
> I'd argue we should do the same for template parameters.
yes it calls both, OK than adding also the `handleDecl`


Repository:
  rC Clang

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

https://reviews.llvm.org/D58293



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

[PATCH] D58314: [Driver] Sync ARM behavior between clang-as and gas.

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

My main concern is that this changes the default behaviour for 
arm-linux-gnueabi and arm-linux-gnueabihf targets. I've put some suggestions on 
what I think the behaviour should be.




Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:277
+  }
+  return "";
+}

I'm a bit worried that we've changed the default behaviour for gnueabi[hf] 
targets here. 
For example with:
```
.text
vmov.32 d13[1], r6 ; Needs VFPv2
vadd.i32 d0, d0, d0 ; Needs Neon
```
I get with --target=armv7a-linux (no environment, -mfloat-abi will disable 
floating point, and neon)
```
clang: warning: unknown platform, assuming -mfloat-abi=soft
neon.s:2:9: error: instruction requires: VFP2
vmov.32 d13[1],r6
^
neon.s:3:9: error: instruction requires: NEON
vadd.i32 d0, d0, d0
^
```
With the target=armv7a-linux-gnueabi armv7a-linux-gnueabihf or explicitly 
adding -mfloat-abi=softfp the integrated assembler will happily assemble it.
GNU needs -mfpu=neon to assemble the file:

```
arm-linux-gnueabihf-as -march=armv7-a neon.s 
neon.s: Assembler messages:
neon.s:2: Error: selected processor does not support ARM mode `vmov.32 
d13[1],r6'
neon.s:3: Error: selected processor does not support ARM mode `vadd.i32 
d0,d0,d0'
```
It is a similar story for armv8 and crypto.

I think we should have something like:
```
if (Triple.isLinux() && getARMSubArchVersionNumber(Triple) >= 8)
   return "crypto-neon-fp-armv8";
if (Triple.isAndroid() || Triple.isLinux() && 
getARMSubArchVersionNumber(Triple) >= 7)
return "neon";
return "";
```



Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:389
+std::string DefaultFPU = getDefaultFPUName(Triple);
+if (DefaultFPU != "") {
+  if (!llvm::ARM::getFPUFeatures(llvm::ARM::parseFPU(DefaultFPU), 
Features))

I'm wondering whether you need this bit of code anymore? In D53121 there needed 
to be a switch between vfpv3-d16 and neon based on Android version. With 
--target=armv7a-linux-android or --target=arm-linux-android -march=armv7a or 
any v7a -mcpu applicable to Android then you'll get feature Neon by default and 
won't need to do this? We could then move getDefaultFPUName out of ARM.cpp



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:692
+}
+if (!hasMOrWaMArg(Args, options::OPT_mfpu_EQ, "-mfpu=")) {
+std::string DefaultFPU = arm::getDefaultFPUName(Triple);

I think we'd not want to do this for -mfloat-abi=soft as this disables the FPU 
in the integrated assembler. It seems like -mfloat-abi has no effect at all on 
the gnu assembler, it will happily assemble neon instructions with 
-mfloat-abi=soft -mfpu=neon.
 



Comment at: clang/test/Driver/linux-as.c:3
 //
 // RUN: %clang -target arm-linux -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \

the target arm-linux (effectively arm-linux-unknown) defaults to 
-mfloat-abi=soft which disables the FPU for the integrated assembler. While 
these test cases are not wrong, the number of v7a + linux targets without an 
FPU using entirely software floating point is likely to be very small. We 
should have some more that have arm-linux-gnueabi and arm-linux-gnueabihf.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58314



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

[PATCH] D58341: [clangd] Index UsingDecls

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 187376.
kadircet marked an inline comment as done.
kadircet added a comment.

- Revert last change


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58341

Files:
  unittests/clangd/SymbolCollectorTests.cpp
  unittests/clangd/SymbolInfoTests.cpp


Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -167,7 +167,8 @@
 )cpp",
   {CreateExpectedSymbolDetails("foo", "", "c:@F@foo#"),
CreateExpectedSymbolDetails("foo", "", "c:@F@foo#b#"),
-   CreateExpectedSymbolDetails("foo", "", "c:@F@foo#I#")}},
+   CreateExpectedSymbolDetails("foo", "", "c:@F@foo#I#"),
+   CreateExpectedSymbolDetails("foo", "bar::", 
"c:@N@bar@UD@foo")}},
   {
   R"cpp( // Multiple symbols returned - implicit conversion
   struct foo {};
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -7,8 +7,12 @@
 
//===--===//
 
 #include "Annotations.h"
+#include "ClangdServer.h"
+#include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/Index.h"
+#include "index/MemIndex.h"
 #include "index/SymbolCollector.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
@@ -50,6 +54,7 @@
 MATCHER_P(Snippet, S, "") {
   return (arg.Name + arg.CompletionSnippetSuffix).str() == S;
 }
+MATCHER_P(CompletionQName, Name, "") { return (arg.Scope + arg.Name) == Name; }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
 MATCHER_P(DeclURI, P, "") {
   return StringRef(arg.CanonicalDeclaration.FileURI) == P;
@@ -325,9 +330,6 @@
 // Namespace alias
 namespace baz = bar;
 
-// FIXME: using declaration is not supported as the IndexAction will ignore
-// implicit declarations (the implicit using shadow declaration) by 
default,
-// and there is no way to customize this behavior at the moment.
 using bar::v2;
 } // namespace foo
   )";
@@ -354,6 +356,7 @@
AllOf(QName("foo::int32_t"), ForCodeCompletion(true)),
AllOf(QName("foo::v1"), ForCodeCompletion(true)),
AllOf(QName("foo::bar::v2"), ForCodeCompletion(true)),
+   AllOf(QName("foo::v2"), ForCodeCompletion(true)),
AllOf(QName("foo::baz"), ForCodeCompletion(true))}));
 }
 
@@ -1118,6 +1121,16 @@
   AllOf(QName("Public"), Not(ImplementationDetail();
 }
 
+TEST_F(SymbolCollectorTest, UsingDecl) {
+  const char *Header = R"(
+  void foo();
+  namespace std {
+using ::foo;
+  })";
+  runSymbolCollector(Header, /**/ "");
+  EXPECT_THAT(Symbols, Contains(QName("std::foo")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -167,7 +167,8 @@
 )cpp",
   {CreateExpectedSymbolDetails("foo", "", "c:@F@foo#"),
CreateExpectedSymbolDetails("foo", "", "c:@F@foo#b#"),
-   CreateExpectedSymbolDetails("foo", "", "c:@F@foo#I#")}},
+   CreateExpectedSymbolDetails("foo", "", "c:@F@foo#I#"),
+   CreateExpectedSymbolDetails("foo", "bar::", "c:@N@bar@UD@foo")}},
   {
   R"cpp( // Multiple symbols returned - implicit conversion
   struct foo {};
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -7,8 +7,12 @@
 //===--===//
 
 #include "Annotations.h"
+#include "ClangdServer.h"
+#include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/Index.h"
+#include "index/MemIndex.h"
 #include "index/SymbolCollector.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
@@ -50,6 +54,7 @@
 MATCHER_P(Snippet, S, "") {
   return (arg.Name + arg.CompletionSnippetSuffix).str() == S;
 }
+MATCHER_P(CompletionQName, Name, "") { return (arg.Scope + arg.Name) == Name; }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
 MATCHER_P(DeclURI, P, "") {
   return StringRef(arg.CanonicalDeclaration.FileURI) == P;
@@ -325,9 +330,6 @@
 // Namespace alias
 namespace baz = bar;
 
-// FIXME: using declaration is not supported as the IndexAction will ignore

[PATCH] D58387: [clangd] Add an option in the code to not display number of fixes

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, ioeric.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Only to the APIs, which are used by our embedders.
We do not plan to add a user-facing option for this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58387

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


Index: clang-tools-extra/clangd/Diagnostics.h
===
--- clang-tools-extra/clangd/Diagnostics.h
+++ clang-tools-extra/clangd/Diagnostics.h
@@ -32,6 +32,10 @@
   /// stage during which the issue was produced, e.g. "Semantic Issue" or 
"Parse
   /// Issue".
   bool SendDiagnosticCategory = false;
+
+  /// If true, Clangd will add a number of available fixes to the diagnostic's
+  /// message.
+  bool DisplayFixesCount = true;
 };
 
 /// Contains basic information about a diagnostic.
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -161,11 +161,11 @@
 ///
 /// dir1/dir2/dir3/../../dir4/header.h:12:23
 /// note: candidate function not viable: requires 3 arguments
-std::string mainMessage(const Diag &D) {
+std::string mainMessage(const Diag &D, bool DisplayFixesCount) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   OS << D.Message;
-  if (!D.Fixes.empty())
+  if (DisplayFixesCount && !D.Fixes.empty())
 OS << " (" << (D.Fixes.size() > 1 ? "fixes" : "fix") << " available)";
   for (auto &Note : D.Notes) {
 OS << "\n\n";
@@ -252,7 +252,7 @@
 
   {
 clangd::Diagnostic Main = FillBasicFields(D);
-Main.message = mainMessage(D);
+Main.message = mainMessage(D, Opts.SendDiagnosticCategory);
 if (Opts.EmbedFixesInDiagnostics) {
   Main.codeActions.emplace();
   for (const auto &Fix : D.Fixes)


Index: clang-tools-extra/clangd/Diagnostics.h
===
--- clang-tools-extra/clangd/Diagnostics.h
+++ clang-tools-extra/clangd/Diagnostics.h
@@ -32,6 +32,10 @@
   /// stage during which the issue was produced, e.g. "Semantic Issue" or "Parse
   /// Issue".
   bool SendDiagnosticCategory = false;
+
+  /// If true, Clangd will add a number of available fixes to the diagnostic's
+  /// message.
+  bool DisplayFixesCount = true;
 };
 
 /// Contains basic information about a diagnostic.
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -161,11 +161,11 @@
 ///
 /// dir1/dir2/dir3/../../dir4/header.h:12:23
 /// note: candidate function not viable: requires 3 arguments
-std::string mainMessage(const Diag &D) {
+std::string mainMessage(const Diag &D, bool DisplayFixesCount) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   OS << D.Message;
-  if (!D.Fixes.empty())
+  if (DisplayFixesCount && !D.Fixes.empty())
 OS << " (" << (D.Fixes.size() > 1 ? "fixes" : "fix") << " available)";
   for (auto &Note : D.Notes) {
 OS << "\n\n";
@@ -252,7 +252,7 @@
 
   {
 clangd::Diagnostic Main = FillBasicFields(D);
-Main.message = mainMessage(D);
+Main.message = mainMessage(D, Opts.SendDiagnosticCategory);
 if (Opts.EmbedFixesInDiagnostics) {
   Main.codeActions.emplace();
   for (const auto &Fix : D.Fixes)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaCast.cpp:2309
+auto DestPointeeTypeWithoutAS = Self.Context.removeAddrSpaceQualType(
+DestPointeeType.getCanonicalType());
+return Self.Context.hasSameType(SrcPointeeTypeWithoutAS,

Anastasia wrote:
> ebevhan wrote:
> > Maybe I'm mistaken, but won't getting the canonical type here drop 
> > qualifiers (like cv) in nested pointers? If so, an addrspace_cast might 
> > strip qualifiers by mistake.
> Yes, indeed I will need to extend this to nested pointers when we are ready. 
> But for now I can try to change this bit... however I am not sure it will 
> work w/o canonical types when we have typedef. I will try to create an 
> example and see.
I checked the canonical type does preserve the qualifiers correctly.

Here is the AST dump of the following C type `mytype const __generic*` where  
`typedef  __generic int* mytype;`.


```
PointerType 0x204d3b0 'const __generic mytype *'
`-QualType 0x204d369 'const __generic mytype' const __generic
  `-TypedefType 0x204d320 'mytype' sugar
|-Typedef 0x204d1b0 'mytype'
`-PointerType 0x204d170 '__generic int *'
  `-QualType 0x204d158 '__generic int' __generic
`-BuiltinType 0x2009750 'int'
```

and it's canonical representation in AST is:

```
PointerType 0x204d380 '__generic int *const __generic *'
`-QualType 0x204d349 '__generic int *const __generic' const __generic
  `-PointerType 0x204d170 '__generic int *'
`-QualType 0x204d158 '__generic int' __generic
  `-BuiltinType 0x2009750 'int'
```

So using canonical type will just simply handling of nested pointer chain by 
avoiding special casing typedefs. We won't loose any qualifiers.


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

https://reviews.llvm.org/D58346



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

[PATCH] D57660: [Sema] SequenceChecker: Handle references and members

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno planned changes to this revision.
riccibruno added a comment.

This needs more work.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57660



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

[PATCH] D58388: [OpenCL] Simplify LLVM IR generated for OpenCL blocks

[Alexey Sotkin via Phabricator via cfe-commits]

AlexeySotkin created this revision.
AlexeySotkin added reviewers: Anastasia, yaxunl, svenvh.
AlexeySotkin added a project: clang.

Emit direct call of block invoke functions when possible, i.e. in case the
block is not passed as a function argument.
Also doing some refactoring of `CodeGenFunction::EmitBlockCallExpr()`


Repository:
  rC Clang

https://reviews.llvm.org/D58388

Files:
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGOpenCLRuntime.cpp
  lib/CodeGen/CGOpenCLRuntime.h
  test/CodeGenOpenCL/blocks.cl
  test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Index: test/CodeGenOpenCL/cl20-device-side-enqueue.cl
===
--- test/CodeGenOpenCL/cl20-device-side-enqueue.cl
+++ test/CodeGenOpenCL/cl20-device-side-enqueue.cl
@@ -312,9 +312,7 @@
   };
 
   // Uses global block literal [[BLG8]] and invoke function [[INVG8]].
-  // COMMON: [[r1:%.*]] = load i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* getelementptr inbounds (%struct.__opencl_block_literal_generic, %struct.__opencl_block_literal_generic addrspace(4)* addrspacecast (%struct.__opencl_block_literal_generic addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BLG8]] to %struct.__opencl_block_literal_generic addrspace(1)*) to %struct.__opencl_block_literal_generic addrspace(4)*), i32 0, i32 2)
-  // COMMON: [[r2:%.*]] = addrspacecast i8 addrspace(4)* [[r1]] to void (i8 addrspace(4)*)*
-  // COMMON: call spir_func void [[r2]](i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BLG8]] to i8 addrspace(1)*) to i8 addrspace(4)*))
+  // COMMON: call spir_func void @__device_side_enqueue_block_invoke_11(i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BLG8]] to i8 addrspace(1)*) to i8 addrspace(4)*))
   block_A();
 
   // Emits global block literal [[BLG8]] and block kernel [[INVGK8]]. [[INVGK8]] calls [[INVG8]].
@@ -333,9 +331,7 @@
   unsigned size = get_kernel_work_group_size(block_A);
 
   // Uses global block literal [[BLG8]] and invoke function [[INVG8]]. Make sure no redundant block literal and invoke functions are emitted.
-  // COMMON: [[r1:%.*]] = load i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* getelementptr inbounds (%struct.__opencl_block_literal_generic, %struct.__opencl_block_literal_generic addrspace(4)* addrspacecast (%struct.__opencl_block_literal_generic addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BLG8]] to %struct.__opencl_block_literal_generic addrspace(1)*) to %struct.__opencl_block_literal_generic addrspace(4)*), i32 0, i32 2)
-  // COMMON: [[r2:%.*]] = addrspacecast i8 addrspace(4)* [[r1]] to void (i8 addrspace(4)*)*
-  // COMMON: call spir_func void [[r2]](i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BLG8]] to i8 addrspace(1)*) to i8 addrspace(4)*))
+  // COMMON: call spir_func void @__device_side_enqueue_block_invoke_11(i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BLG8]] to i8 addrspace(1)*) to i8 addrspace(4)*))
   block_A();
 
   void (^block_C)(void) = ^{
Index: test/CodeGenOpenCL/blocks.cl
===
--- test/CodeGenOpenCL/blocks.cl
+++ test/CodeGenOpenCL/blocks.cl
@@ -39,11 +39,8 @@
   // SPIR: %[[blk_gen_ptr:.*]] = addrspacecast %struct.__opencl_block_literal_generic* %[[blk_ptr]] to %struct.__opencl_block_literal_generic addrspace(4)*
   // SPIR: store %struct.__opencl_block_literal_generic addrspace(4)* %[[blk_gen_ptr]], %struct.__opencl_block_literal_generic addrspace(4)** %[[block_B:.*]],
   // SPIR: %[[block_literal:.*]] = load %struct.__opencl_block_literal_generic addrspace(4)*, %struct.__opencl_block_literal_generic addrspace(4)** %[[block_B]]
-  // SPIR: %[[invoke_addr:.*]] = getelementptr inbounds %struct.__opencl_block_literal_generic, %struct.__opencl_block_literal_generic addrspace(4)* %[[block_literal]], i32 0, i32 2
   // SPIR: %[[blk_gen_ptr:.*]] = bitcast %struct.__opencl_block_literal_generic addrspace(4)* %[[block_literal]] to i8 addrspace(4)*
-  // SPIR: %[[invoke_func_ptr:.*]] = load i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* %[[invoke_addr]]
-  // SPIR: %[[invoke_func:.*]] = addrspacecast i8 addrspace(4)* %[[invoke_func_ptr]] to i32 (i8 addrspace(4)*)*
-  // SPIR: call {{.*}}i32 %[[invoke_func]](i8 addrspace(4)* %[[blk_gen_ptr]])
+  // SPIR: call {{.*}}i32 @__foo_block_invoke(i8 addrspace(4)* %[[blk_gen_ptr]])
   // AMDGCN: %[[block_invoke:.*]] = getelementptr inbounds <{ i32, i32, i8*, i32 }>, <{ i32, i32, i8*, i32 }> addrspace(5)* %[[block:.*]], i32 0, i32 2
   // AMDGCN: store i8* bitcast (i32 (i8*)* @__foo_block_invoke to i8*), i8* addrspace(5)* %[[block_invoke]]
   // AMDGCN: %[[block_captured:.*]] = getelementptr inbounds <{ i32, i32, i8*, i32 }>, <{ i32, i32, i8*, i32 }> addrspace(5)* %[[block]], i32 0, i32 3
@

[PATCH] D58343: Enablement for AMD znver2 architecture - skeleton patch

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

I think you uploaded the clang patch into the llvm review?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58343



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

[clang-tools-extra] r354349 - [clangd] Add an option in the code to not display number of fixes

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Feb 19 08:50:37 2019
New Revision: 354349

URL: http://llvm.org/viewvc/llvm-project?rev=354349&view=rev
Log:
[clangd] Add an option in the code to not display number of fixes

Summary:
Only to the APIs, which are used by our embedders.
We do not plan to add a user-facing option for this.

Reviewers: sammccall, ioeric

Reviewed By: sammccall

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

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=354349&r1=354348&r2=354349&view=diff
==
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Tue Feb 19 08:50:37 2019
@@ -161,11 +161,11 @@ std::string capitalize(std::string Messa
 ///
 /// dir1/dir2/dir3/../../dir4/header.h:12:23
 /// note: candidate function not viable: requires 3 arguments
-std::string mainMessage(const Diag &D) {
+std::string mainMessage(const Diag &D, bool DisplayFixesCount) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   OS << D.Message;
-  if (!D.Fixes.empty())
+  if (DisplayFixesCount && !D.Fixes.empty())
 OS << " (" << (D.Fixes.size() > 1 ? "fixes" : "fix") << " available)";
   for (auto &Note : D.Notes) {
 OS << "\n\n";
@@ -252,7 +252,7 @@ void toLSPDiags(
 
   {
 clangd::Diagnostic Main = FillBasicFields(D);
-Main.message = mainMessage(D);
+Main.message = mainMessage(D, Opts.DisplayFixesCount);
 if (Opts.EmbedFixesInDiagnostics) {
   Main.codeActions.emplace();
   for (const auto &Fix : D.Fixes)

Modified: clang-tools-extra/trunk/clangd/Diagnostics.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.h?rev=354349&r1=354348&r2=354349&view=diff
==
--- clang-tools-extra/trunk/clangd/Diagnostics.h (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.h Tue Feb 19 08:50:37 2019
@@ -32,6 +32,10 @@ struct ClangdDiagnosticOptions {
   /// stage during which the issue was produced, e.g. "Semantic Issue" or 
"Parse
   /// Issue".
   bool SendDiagnosticCategory = false;
+
+  /// If true, Clangd will add a number of available fixes to the diagnostic's
+  /// message.
+  bool DisplayFixesCount = true;
 };
 
 /// Contains basic information about a diagnostic.


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

[PATCH] D58387: [clangd] Add an option in the code to not display number of fixes

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354349: [clangd] Add an option in the code to not display 
number of fixes (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58387?vs=187377&id=187383#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58387

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


Index: clang-tools-extra/trunk/clangd/Diagnostics.cpp
===
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp
@@ -161,11 +161,11 @@
 ///
 /// dir1/dir2/dir3/../../dir4/header.h:12:23
 /// note: candidate function not viable: requires 3 arguments
-std::string mainMessage(const Diag &D) {
+std::string mainMessage(const Diag &D, bool DisplayFixesCount) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   OS << D.Message;
-  if (!D.Fixes.empty())
+  if (DisplayFixesCount && !D.Fixes.empty())
 OS << " (" << (D.Fixes.size() > 1 ? "fixes" : "fix") << " available)";
   for (auto &Note : D.Notes) {
 OS << "\n\n";
@@ -252,7 +252,7 @@
 
   {
 clangd::Diagnostic Main = FillBasicFields(D);
-Main.message = mainMessage(D);
+Main.message = mainMessage(D, Opts.DisplayFixesCount);
 if (Opts.EmbedFixesInDiagnostics) {
   Main.codeActions.emplace();
   for (const auto &Fix : D.Fixes)
Index: clang-tools-extra/trunk/clangd/Diagnostics.h
===
--- clang-tools-extra/trunk/clangd/Diagnostics.h
+++ clang-tools-extra/trunk/clangd/Diagnostics.h
@@ -32,6 +32,10 @@
   /// stage during which the issue was produced, e.g. "Semantic Issue" or 
"Parse
   /// Issue".
   bool SendDiagnosticCategory = false;
+
+  /// If true, Clangd will add a number of available fixes to the diagnostic's
+  /// message.
+  bool DisplayFixesCount = true;
 };
 
 /// Contains basic information about a diagnostic.


Index: clang-tools-extra/trunk/clangd/Diagnostics.cpp
===
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp
@@ -161,11 +161,11 @@
 ///
 /// dir1/dir2/dir3/../../dir4/header.h:12:23
 /// note: candidate function not viable: requires 3 arguments
-std::string mainMessage(const Diag &D) {
+std::string mainMessage(const Diag &D, bool DisplayFixesCount) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   OS << D.Message;
-  if (!D.Fixes.empty())
+  if (DisplayFixesCount && !D.Fixes.empty())
 OS << " (" << (D.Fixes.size() > 1 ? "fixes" : "fix") << " available)";
   for (auto &Note : D.Notes) {
 OS << "\n\n";
@@ -252,7 +252,7 @@
 
   {
 clangd::Diagnostic Main = FillBasicFields(D);
-Main.message = mainMessage(D);
+Main.message = mainMessage(D, Opts.DisplayFixesCount);
 if (Opts.EmbedFixesInDiagnostics) {
   Main.codeActions.emplace();
   for (const auto &Fix : D.Fixes)
Index: clang-tools-extra/trunk/clangd/Diagnostics.h
===
--- clang-tools-extra/trunk/clangd/Diagnostics.h
+++ clang-tools-extra/trunk/clangd/Diagnostics.h
@@ -32,6 +32,10 @@
   /// stage during which the issue was produced, e.g. "Semantic Issue" or "Parse
   /// Issue".
   bool SendDiagnosticCategory = false;
+
+  /// If true, Clangd will add a number of available fixes to the diagnostic's
+  /// message.
+  bool DisplayFixesCount = true;
 };
 
 /// Contains basic information about a diagnostic.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r354351 - Remove extraneous space in MSVC-style diagnostic output

[Hans Wennborg via cfe-commits]

Author: hans
Date: Tue Feb 19 08:58:25 2019
New Revision: 354351

URL: http://llvm.org/viewvc/llvm-project?rev=354351&view=rev
Log:
Remove extraneous space in MSVC-style diagnostic output

There was an extra space between the file location and the diagnostic
message:

  /tmp/a.c(1,12):  warning: unused parameter 'unused'

the tests didn't catch this due to FileCheck not running in --strict-whitespace 
mode.

Reported by Marco: 
http://lists.llvm.org/pipermail/cfe-dev/2019-February/061326.html

Differential revision: https://reviews.llvm.org/D58377

Modified:
cfe/trunk/lib/Frontend/TextDiagnostic.cpp
cfe/trunk/test/Misc/diag-format.c

Modified: cfe/trunk/lib/Frontend/TextDiagnostic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/TextDiagnostic.cpp?rev=354351&r1=354350&r2=354351&view=diff
==
--- cfe/trunk/lib/Frontend/TextDiagnostic.cpp (original)
+++ cfe/trunk/lib/Frontend/TextDiagnostic.cpp Tue Feb 19 08:58:25 2019
@@ -837,7 +837,7 @@ void TextDiagnostic::emitDiagnosticLoc(F
 if (LangOpts.MSCompatibilityVersion &&
 !LangOpts.isCompatibleWithMSVC(LangOptions::MSVC2015))
   OS << ' ';
-OS << ": ";
+OS << ':';
 break;
   }
 

Modified: cfe/trunk/test/Misc/diag-format.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/diag-format.c?rev=354351&r1=354350&r2=354351&view=diff
==
--- cfe/trunk/test/Misc/diag-format.c (original)
+++ cfe/trunk/test/Misc/diag-format.c Tue Feb 19 08:58:25 2019
@@ -1,30 +1,30 @@
-// RUN: %clang -fsyntax-only  %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
-// RUN: %clang -fsyntax-only -fdiagnostics-format=clang %s 2>&1 | FileCheck %s 
-check-prefix=DEFAULT
-// RUN: %clang -fsyntax-only -fdiagnostics-format=clang -target 
x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang -fsyntax-only  %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=DEFAULT
+// RUN: %clang -fsyntax-only -fdiagnostics-format=clang %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=DEFAULT
+// RUN: %clang -fsyntax-only -fdiagnostics-format=clang -target 
x86_64-pc-win32 %s 2>&1 | FileCheck %s --strict-whitespace -check-prefix=DEFAULT
 //
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300  %s 
2>&1 | FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00  %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1800 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2013
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 
%s 2>&1 | FileCheck %s -check-prefix=MSVC
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1900 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2015
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 -fshow-column %s 2>&1 
| FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1800 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2013
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 
-fshow-column %s 2>&1 | FileCheck %s -check-prefix=MSVC
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1900 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2015
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300  %s 
2>&1 | FileCheck %s --strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00  %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1

[PATCH] D58344: Enablement for AMD znver2 architecture - skeleton

[Ganesh Gopalasubramanian via Phabricator via cfe-commits]

GGanesh updated this revision to Diff 187387.

Repository:
  rC Clang

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

https://reviews.llvm.org/D58344

Files:
  include/clang/Basic/X86Target.def
  lib/Basic/Targets/X86.cpp
  test/CodeGen/target-builtin-noerror.c
  test/Driver/x86-march.c
  test/Frontend/x86-target-cpu.c
  test/Misc/target-invalid-cpu-note.c
  test/Preprocessor/predefined-arch-macros.c

Index: test/Preprocessor/predefined-arch-macros.c
===
--- test/Preprocessor/predefined-arch-macros.c
+++ test/Preprocessor/predefined-arch-macros.c
@@ -2676,6 +2676,100 @@
 // CHECK_ZNVER1_M64: #define __znver1 1
 // CHECK_ZNVER1_M64: #define __znver1__ 1
 
+// RUN: %clang -march=znver2 -m32 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_ZNVER2_M32
+// CHECK_ZNVER2_M32-NOT: #define __3dNOW_A__ 1
+// CHECK_ZNVER2_M32-NOT: #define __3dNOW__ 1
+// CHECK_ZNVER2_M32: #define __ADX__ 1
+// CHECK_ZNVER2_M32: #define __AES__ 1
+// CHECK_ZNVER2_M32: #define __AVX2__ 1
+// CHECK_ZNVER2_M32: #define __AVX__ 1
+// CHECK_ZNVER2_M32: #define __BMI2__ 1
+// CHECK_ZNVER2_M32: #define __BMI__ 1
+// CHECK_ZNVER2_M32: #define __CLFLUSHOPT__ 1
+// CHECK_ZNVER2_M32: #define __CLWB__ 1
+// CHECK_ZNVER2_M32: #define __CLZERO__ 1
+// CHECK_ZNVER2_M32: #define __F16C__ 1
+// CHECK_ZNVER2_M32: #define __FMA__ 1
+// CHECK_ZNVER2_M32: #define __FSGSBASE__ 1
+// CHECK_ZNVER2_M32: #define __LZCNT__ 1
+// CHECK_ZNVER2_M32: #define __MMX__ 1
+// CHECK_ZNVER2_M32: #define __PCLMUL__ 1
+// CHECK_ZNVER2_M32: #define __POPCNT__ 1
+// CHECK_ZNVER2_M32: #define __PRFCHW__ 1
+// CHECK_ZNVER2_M32: #define __RDPID__ 1
+// CHECK_ZNVER2_M32: #define __RDRND__ 1
+// CHECK_ZNVER2_M32: #define __RDSEED__ 1
+// CHECK_ZNVER2_M32: #define __SHA__ 1
+// CHECK_ZNVER2_M32: #define __SSE2_MATH__ 1
+// CHECK_ZNVER2_M32: #define __SSE2__ 1
+// CHECK_ZNVER2_M32: #define __SSE3__ 1
+// CHECK_ZNVER2_M32: #define __SSE4A__ 1
+// CHECK_ZNVER2_M32: #define __SSE4_1__ 1
+// CHECK_ZNVER2_M32: #define __SSE4_2__ 1
+// CHECK_ZNVER2_M32: #define __SSE_MATH__ 1
+// CHECK_ZNVER2_M32: #define __SSE__ 1
+// CHECK_ZNVER2_M32: #define __SSSE3__ 1
+// CHECK_ZNVER2_M32: #define __WBNOINVD__ 1
+// CHECK_ZNVER2_M32: #define __XSAVEC__ 1
+// CHECK_ZNVER2_M32: #define __XSAVEOPT__ 1
+// CHECK_ZNVER2_M32: #define __XSAVES__ 1
+// CHECK_ZNVER2_M32: #define __XSAVE__ 1
+// CHECK_ZNVER2_M32: #define __i386 1
+// CHECK_ZNVER2_M32: #define __i386__ 1
+// CHECK_ZNVER2_M32: #define __tune_znver2__ 1
+// CHECK_ZNVER2_M32: #define __znver2 1
+// CHECK_ZNVER2_M32: #define __znver2__ 1
+
+// RUN: %clang -march=znver2 -m64 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_ZNVER2_M64
+// CHECK_ZNVER2_M64-NOT: #define __3dNOW_A__ 1
+// CHECK_ZNVER2_M64-NOT: #define __3dNOW__ 1
+// CHECK_ZNVER2_M64: #define __ADX__ 1
+// CHECK_ZNVER2_M64: #define __AES__ 1
+// CHECK_ZNVER2_M64: #define __AVX2__ 1
+// CHECK_ZNVER2_M64: #define __AVX__ 1
+// CHECK_ZNVER2_M64: #define __BMI2__ 1
+// CHECK_ZNVER2_M64: #define __BMI__ 1
+// CHECK_ZNVER2_M64: #define __CLFLUSHOPT__ 1
+// CHECK_ZNVER2_M64: #define __CLWB__ 1
+// CHECK_ZNVER2_M64: #define __CLZERO__ 1
+// CHECK_ZNVER2_M64: #define __F16C__ 1
+// CHECK_ZNVER2_M64: #define __FMA__ 1
+// CHECK_ZNVER2_M64: #define __FSGSBASE__ 1
+// CHECK_ZNVER2_M64: #define __LZCNT__ 1
+// CHECK_ZNVER2_M64: #define __MMX__ 1
+// CHECK_ZNVER2_M64: #define __PCLMUL__ 1
+// CHECK_ZNVER2_M64: #define __POPCNT__ 1
+// CHECK_ZNVER2_M64: #define __PRFCHW__ 1
+// CHECK_ZNVER2_M64: #define __RDPID__ 1
+// CHECK_ZNVER2_M64: #define __RDRND__ 1
+// CHECK_ZNVER2_M64: #define __RDSEED__ 1
+// CHECK_ZNVER2_M64: #define __SHA__ 1
+// CHECK_ZNVER2_M64: #define __SSE2_MATH__ 1
+// CHECK_ZNVER2_M64: #define __SSE2__ 1
+// CHECK_ZNVER2_M64: #define __SSE3__ 1
+// CHECK_ZNVER2_M64: #define __SSE4A__ 1
+// CHECK_ZNVER2_M64: #define __SSE4_1__ 1
+// CHECK_ZNVER2_M64: #define __SSE4_2__ 1
+// CHECK_ZNVER2_M64: #define __SSE_MATH__ 1
+// CHECK_ZNVER2_M64: #define __SSE__ 1
+// CHECK_ZNVER2_M64: #define __SSSE3__ 1
+// CHECK_ZNVER2_M64: #define __WBNOINVD__ 1
+// CHECK_ZNVER2_M64: #define __XSAVEC__ 1
+// CHECK_ZNVER2_M64: #define __XSAVEOPT__ 1
+// CHECK_ZNVER2_M64: #define __XSAVES__ 1
+// CHECK_ZNVER2_M64: #define __XSAVE__ 1
+// CHECK_ZNVER2_M64: #define __amd64 1
+// CHECK_ZNVER2_M64: #define __amd64__ 1
+// CHECK_ZNVER2_M64: #define __tune_znver2__ 1
+// CHECK_ZNVER2_M64: #define __x86_64 1
+// CHECK_ZNVER2_M64: #define __x86_64__ 1
+// CHECK_ZNVER2_M64: #define __znver2 1
+// CHECK_ZNVER2_M64: #define __znver2__ 1
+
 // End X86/GCC/Linux tests --
 
 // Begin PPC/GCC/Linux tests 
Index: test/Misc/target-invalid-cpu-note.c
===
--- test/Misc/target-invalid-cpu-note.c

[PATCH] D58343: Enablement for AMD znver2 architecture - skeleton patch

[Ganesh Gopalasubramanian via Phabricator via cfe-commits]

GGanesh updated this revision to Diff 187386.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58343

Files:
  include/llvm/Support/X86TargetParser.def
  lib/Support/Host.cpp
  lib/Target/X86/X86.td
  test/CodeGen/X86/cpus-amd.ll
  test/CodeGen/X86/lzcnt-zext-cmp.ll
  test/CodeGen/X86/slow-unaligned-mem.ll
  test/CodeGen/X86/x86-64-double-shifts-var.ll

Index: test/CodeGen/X86/x86-64-double-shifts-var.ll
===
--- test/CodeGen/X86/x86-64-double-shifts-var.ll
+++ test/CodeGen/X86/x86-64-double-shifts-var.ll
@@ -13,8 +13,9 @@
 ; RUN: llc < %s -mtriple=x86_64-- -mcpu=bdver3 | FileCheck %s
 ; RUN: llc < %s -mtriple=x86_64-- -mcpu=bdver4 | FileCheck %s
 ; RUN: llc < %s -mtriple=x86_64-- -mcpu=znver1 | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-- -mcpu=znver2 | FileCheck %s
 
-; Verify that for the X86_64 processors that are known to have poor latency 
+; Verify that for the X86_64 processors that are known to have poor latency
 ; double precision shift instructions we do not generate 'shld' or 'shrd'
 ; instructions.
 
@@ -25,7 +26,7 @@
 
 define i64 @lshift(i64 %a, i64 %b, i32 %c) nounwind readnone {
 entry:
-; CHECK-NOT: shld 
+; CHECK-NOT: shld
   %sh_prom = zext i32 %c to i64
   %shl = shl i64 %a, %sh_prom
   %sub = sub nsw i32 64, %c
Index: test/CodeGen/X86/slow-unaligned-mem.ll
===
--- test/CodeGen/X86/slow-unaligned-mem.ll
+++ test/CodeGen/X86/slow-unaligned-mem.ll
@@ -47,6 +47,7 @@
 ; RUN: llc < %s -mtriple=i386-unknown-unknown -mcpu=bdver32>&1 | FileCheck %s --check-prefix=FAST
 ; RUN: llc < %s -mtriple=i386-unknown-unknown -mcpu=bdver42>&1 | FileCheck %s --check-prefix=FAST
 ; RUN: llc < %s -mtriple=i386-unknown-unknown -mcpu=znver12>&1 | FileCheck %s --check-prefix=FAST
+; RUN: llc < %s -mtriple=i386-unknown-unknown -mcpu=znver22>&1 | FileCheck %s --check-prefix=FAST
 
 ; Other chips with slow unaligned memory accesses
 
Index: test/CodeGen/X86/lzcnt-zext-cmp.ll
===
--- test/CodeGen/X86/lzcnt-zext-cmp.ll
+++ test/CodeGen/X86/lzcnt-zext-cmp.ll
@@ -5,6 +5,8 @@
 ; RUN: llc < %s -mtriple=x86_64-pc-linux -mcpu=btver2 -mattr=-fast-lzcnt | FileCheck --check-prefix=ALL --check-prefix=NOFASTLZCNT %s
 ; RUN: llc < %s -mtriple=x86_64-pc-linux -mcpu=znver1 | FileCheck --check-prefix=ALL --check-prefix=FASTLZCNT %s
 ; RUN: llc < %s -mtriple=x86_64-pc-linux -mcpu=znver1 -mattr=-fast-lzcnt | FileCheck --check-prefix=ALL --check-prefix=NOFASTLZCNT %s
+; RUN: llc < %s -mtriple=x86_64-pc-linux -mcpu=znver2 | FileCheck --check-prefix=ALL --check-prefix=FASTLZCNT %s
+; RUN: llc < %s -mtriple=x86_64-pc-linux -mcpu=znver2 -mattr=-fast-lzcnt | FileCheck --check-prefix=ALL --check-prefix=NOFASTLZCNT %s
 
 ; Test one 32-bit input, output is 32-bit, no transformations expected.
 define i32 @test_zext_cmp0(i32 %a) {
Index: test/CodeGen/X86/cpus-amd.ll
===
--- test/CodeGen/X86/cpus-amd.ll
+++ test/CodeGen/X86/cpus-amd.ll
@@ -26,6 +26,7 @@
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=btver1 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=btver2 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=znver1 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=znver2 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 define void @foo() {
   ret void
Index: lib/Target/X86/X86.td
===
--- lib/Target/X86/X86.td
+++ lib/Target/X86/X86.td
@@ -1144,15 +1144,14 @@
   FeatureMacroFusion
 ]>;
 
-// Znver1
-def: ProcessorModel<"znver1", Znver1Model, [
+// AMD Zen Processors common ISAs
+def ZNFeatures : ProcessorFeatures<[], [
   FeatureADX,
   FeatureAES,
   FeatureAVX2,
   FeatureBMI,
   FeatureBMI2,
   FeatureCLFLUSHOPT,
-  FeatureCLZERO,
   FeatureCMOV,
   Feature64Bit,
   FeatureCMPXCHG16B,
@@ -1184,6 +1183,21 @@
   FeatureXSAVEOPT,
   FeatureXSAVES]>;
 
+class Znver1Proc : ProcModel;
+def : Znver1Proc<"znver1">;
+
+class Znver2Proc : ProcModel;
+def : Znver2Proc<"znver2">;
+
 def : Proc<"geode",   [FeatureX87, FeatureSlowUAMem16, Feature3DNowA]>;
 
 def : Proc<"winchip-c6",  [FeatureX87, FeatureSlowUAMem16, FeatureMMX]>;
Index: lib/Support/Host.cpp
===
--- lib/Support/Host.cpp
+++ lib/Support/Host.cpp
@@ -916,7 +916,14 @@
 break; // "btver2"
   case 23:
 *Type = X86::AMDFAM17H;
-*Subtype = X86::AMDFAM17H_ZNVER1;
+if (Model >= 0x30 && Model <= 0x3f) {
+  *Su

[PATCH] D58377: Remove extraneous space in MSVC-style diagnostic output

[Hans Wennborg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC354351: Remove extraneous space in MSVC-style diagnostic 
output (authored by hans, committed by ).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D58377?vs=187320&id=187388#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58377

Files:
  lib/Frontend/TextDiagnostic.cpp
  test/Misc/diag-format.c


Index: test/Misc/diag-format.c
===
--- test/Misc/diag-format.c
+++ test/Misc/diag-format.c
@@ -1,30 +1,30 @@
-// RUN: %clang -fsyntax-only  %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
-// RUN: %clang -fsyntax-only -fdiagnostics-format=clang %s 2>&1 | FileCheck %s 
-check-prefix=DEFAULT
-// RUN: %clang -fsyntax-only -fdiagnostics-format=clang -target 
x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang -fsyntax-only  %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=DEFAULT
+// RUN: %clang -fsyntax-only -fdiagnostics-format=clang %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=DEFAULT
+// RUN: %clang -fsyntax-only -fdiagnostics-format=clang -target 
x86_64-pc-win32 %s 2>&1 | FileCheck %s --strict-whitespace -check-prefix=DEFAULT
 //
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300  %s 
2>&1 | FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00  %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1800 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2013
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 
%s 2>&1 | FileCheck %s -check-prefix=MSVC
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1900 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2015
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 -fshow-column %s 2>&1 
| FileCheck %s -check-prefix=MSVC2010
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1800 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2013
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 
-fshow-column %s 2>&1 | FileCheck %s -check-prefix=MSVC
-// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1900 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
-check-prefix=MSVC2015
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300  %s 
2>&1 | FileCheck %s --strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00  %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1800 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=MSVC2013
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 
%s 2>&1 | FileCheck %s --strict-whitespace -check-prefix=MSVC
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1900 
-target x86_64-pc-win32 %s 2>&1 | FileCheck %s --strict-whitespace 
-check-prefix=MSVC2015
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc 
-fms-compatibility-version=13.00 -target x86_64-pc-win32 -fshow-column %s 2>&1 
| FileCheck %s --strict-whitespace -check-prefix=MSVC2010
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1800 
-target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s 
--strict-whitespace -check-prefix=MSVC2013
+// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 
-fshow-column %s 2>&1 | FileCheck %s --strict-whitespace -check-prefix=MSVC
+// RUN: %clang -fsyntax-only -

[PATCH] D58343: Enablement for AMD znver2 architecture - skeleton patch

[Ganesh Gopalasubramanian via Phabricator via cfe-commits]

GGanesh updated this revision to Diff 187389.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58343

Files:
  include/llvm/Support/X86TargetParser.def
  lib/Support/Host.cpp
  lib/Target/X86/X86.td
  test/CodeGen/X86/cpus-amd.ll
  test/CodeGen/X86/lzcnt-zext-cmp.ll
  test/CodeGen/X86/slow-unaligned-mem.ll
  test/CodeGen/X86/x86-64-double-shifts-var.ll

Index: test/CodeGen/X86/x86-64-double-shifts-var.ll
===
--- test/CodeGen/X86/x86-64-double-shifts-var.ll
+++ test/CodeGen/X86/x86-64-double-shifts-var.ll
@@ -13,8 +13,9 @@
 ; RUN: llc < %s -mtriple=x86_64-- -mcpu=bdver3 | FileCheck %s
 ; RUN: llc < %s -mtriple=x86_64-- -mcpu=bdver4 | FileCheck %s
 ; RUN: llc < %s -mtriple=x86_64-- -mcpu=znver1 | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-- -mcpu=znver2 | FileCheck %s
 
-; Verify that for the X86_64 processors that are known to have poor latency 
+; Verify that for the X86_64 processors that are known to have poor latency
 ; double precision shift instructions we do not generate 'shld' or 'shrd'
 ; instructions.
 
@@ -25,7 +26,7 @@
 
 define i64 @lshift(i64 %a, i64 %b, i32 %c) nounwind readnone {
 entry:
-; CHECK-NOT: shld 
+; CHECK-NOT: shld
   %sh_prom = zext i32 %c to i64
   %shl = shl i64 %a, %sh_prom
   %sub = sub nsw i32 64, %c
Index: test/CodeGen/X86/slow-unaligned-mem.ll
===
--- test/CodeGen/X86/slow-unaligned-mem.ll
+++ test/CodeGen/X86/slow-unaligned-mem.ll
@@ -47,6 +47,7 @@
 ; RUN: llc < %s -mtriple=i386-unknown-unknown -mcpu=bdver32>&1 | FileCheck %s --check-prefix=FAST
 ; RUN: llc < %s -mtriple=i386-unknown-unknown -mcpu=bdver42>&1 | FileCheck %s --check-prefix=FAST
 ; RUN: llc < %s -mtriple=i386-unknown-unknown -mcpu=znver12>&1 | FileCheck %s --check-prefix=FAST
+; RUN: llc < %s -mtriple=i386-unknown-unknown -mcpu=znver22>&1 | FileCheck %s --check-prefix=FAST
 
 ; Other chips with slow unaligned memory accesses
 
Index: test/CodeGen/X86/lzcnt-zext-cmp.ll
===
--- test/CodeGen/X86/lzcnt-zext-cmp.ll
+++ test/CodeGen/X86/lzcnt-zext-cmp.ll
@@ -5,6 +5,8 @@
 ; RUN: llc < %s -mtriple=x86_64-pc-linux -mcpu=btver2 -mattr=-fast-lzcnt | FileCheck --check-prefix=ALL --check-prefix=NOFASTLZCNT %s
 ; RUN: llc < %s -mtriple=x86_64-pc-linux -mcpu=znver1 | FileCheck --check-prefix=ALL --check-prefix=FASTLZCNT %s
 ; RUN: llc < %s -mtriple=x86_64-pc-linux -mcpu=znver1 -mattr=-fast-lzcnt | FileCheck --check-prefix=ALL --check-prefix=NOFASTLZCNT %s
+; RUN: llc < %s -mtriple=x86_64-pc-linux -mcpu=znver2 | FileCheck --check-prefix=ALL --check-prefix=FASTLZCNT %s
+; RUN: llc < %s -mtriple=x86_64-pc-linux -mcpu=znver2 -mattr=-fast-lzcnt | FileCheck --check-prefix=ALL --check-prefix=NOFASTLZCNT %s
 
 ; Test one 32-bit input, output is 32-bit, no transformations expected.
 define i32 @test_zext_cmp0(i32 %a) {
Index: test/CodeGen/X86/cpus-amd.ll
===
--- test/CodeGen/X86/cpus-amd.ll
+++ test/CodeGen/X86/cpus-amd.ll
@@ -26,6 +26,7 @@
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=btver1 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=btver2 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=znver1 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=znver2 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 define void @foo() {
   ret void
Index: lib/Target/X86/X86.td
===
--- lib/Target/X86/X86.td
+++ lib/Target/X86/X86.td
@@ -1144,8 +1144,8 @@
   FeatureMacroFusion
 ]>;
 
-// Znver1
-def: ProcessorModel<"znver1", Znver1Model, [
+// AMD Zen Processors common ISAs
+def ZNFeatures : ProcessorFeatures<[], [
   FeatureADX,
   FeatureAES,
   FeatureAVX2,
@@ -1184,6 +1184,19 @@
   FeatureXSAVEOPT,
   FeatureXSAVES]>;
 
+class Znver1Proc : ProcModel;
+def : Znver1Proc<"znver1">;
+
+class Znver2Proc : ProcModel;
+def : Znver2Proc<"znver2">;
+
 def : Proc<"geode",   [FeatureX87, FeatureSlowUAMem16, Feature3DNowA]>;
 
 def : Proc<"winchip-c6",  [FeatureX87, FeatureSlowUAMem16, FeatureMMX]>;
Index: lib/Support/Host.cpp
===
--- lib/Support/Host.cpp
+++ lib/Support/Host.cpp
@@ -916,7 +916,14 @@
 break; // "btver2"
   case 23:
 *Type = X86::AMDFAM17H;
-*Subtype = X86::AMDFAM17H_ZNVER1;
+if (Model >= 0x30 && Model <= 0x3f) {
+  *Subtype = X86::AMDFAM17H_ZNVER2;
+  break; // "znver2"; 30h-3fh: Zen2
+}
+if (Model <= 0x0f) {
+  *Subtype = X86::

[PATCH] D58343: Enablement for AMD znver2 architecture - skeleton patch

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58343



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

[PATCH] D58292: Add support for importing ChooseExpr AST nodes.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder updated this revision to Diff 187393.
tmroeder marked 6 inline comments as done.
tmroeder added a comment.

Thanks for the review and the suggestions for improving the tests.

This update cleans up the tests as suggested.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58292

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  test/ASTMerge/choose-expr/Inputs/choose.c
  test/ASTMerge/choose-expr/test.c
  unittests/AST/ASTImporterTest.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -754,6 +754,11 @@
   EXPECT_TRUE(matches("int* i = nullptr;", cxxNullPtrLiteralExpr()));
 }
 
+TEST(Matcher, ChooseExpr) {
+  EXPECT_TRUE(matchesC("void f() { (void)__builtin_choose_expr(1, 2, 3); }",
+   chooseExpr()));
+}
+
 TEST(Matcher, GNUNullExpr) {
   EXPECT_TRUE(matches("int* i = __null;", gnuNullExpr()));
 }
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -563,6 +563,15 @@
   stringLiteral(hasType(asString("const char [7]"));
 }
 
+TEST_P(ImportExpr, ImportChooseExpr) {
+  MatchVerifier Verifier;
+
+  testImport(
+"void declToImport() { __builtin_choose_expr(1, 2, 3); }",
+Lang_C, "", Lang_C, Verifier,
+functionDecl(hasDescendant(chooseExpr(;
+}
+
 TEST_P(ImportExpr, ImportGNUNullExpr) {
   MatchVerifier Verifier;
   testImport(
Index: test/ASTMerge/choose-expr/test.c
===
--- /dev/null
+++ test/ASTMerge/choose-expr/test.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -std=c11 -emit-pch -o %t.ast %S/Inputs/choose.c
+// RUN: %clang_cc1 -std=c11 -ast-merge %t.ast -fsyntax-only -verify %s
+// expected-no-diagnostics
+
Index: test/ASTMerge/choose-expr/Inputs/choose.c
===
--- /dev/null
+++ test/ASTMerge/choose-expr/Inputs/choose.c
@@ -0,0 +1,2 @@
+_Static_assert(__builtin_choose_expr(1, 1, 0), "Incorrect semantics of __builtin_choose_expr");
+_Static_assert(__builtin_choose_expr(0, 0, 1), "Incorrect semantics of __builtin_choose_expr");
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -147,6 +147,7 @@
   REGISTER_MATCHER(caseStmt);
   REGISTER_MATCHER(castExpr);
   REGISTER_MATCHER(characterLiteral);
+  REGISTER_MATCHER(chooseExpr);
   REGISTER_MATCHER(classTemplateDecl);
   REGISTER_MATCHER(classTemplateSpecializationDecl);
   REGISTER_MATCHER(complexType);
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -727,6 +727,7 @@
 compoundLiteralExpr;
 const internal::VariadicDynCastAllOfMatcher
 cxxNullPtrLiteralExpr;
+const internal::VariadicDynCastAllOfMatcher chooseExpr;
 const internal::VariadicDynCastAllOfMatcher gnuNullExpr;
 const internal::VariadicDynCastAllOfMatcher atomicExpr;
 const internal::VariadicDynCastAllOfMatcher stmtExpr;
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -552,6 +552,7 @@
 // Importing expressions
 ExpectedStmt VisitExpr(Expr *E);
 ExpectedStmt VisitVAArgExpr(VAArgExpr *E);
+ExpectedStmt VisitChooseExpr(ChooseExpr *E);
 ExpectedStmt VisitGNUNullExpr(GNUNullExpr *E);
 ExpectedStmt VisitPredefinedExpr(PredefinedExpr *E);
 ExpectedStmt VisitDeclRefExpr(DeclRefExpr *E);
@@ -6135,6 +6136,35 @@
   E->isMicrosoftABI());
 }
 
+ExpectedStmt ASTNodeImporter::VisitChooseExpr(ChooseExpr *E) {
+  auto Imp = importSeq(E->getCond(), E->getLHS(), E->getRHS(),
+   E->getBuiltinLoc(), E->getRParenLoc(), E->getType());
+  if (!Imp)
+return Imp.takeError();
+
+  Expr *ToCond;
+  Expr *ToLHS;
+  Expr *ToRHS;
+  SourceLocation ToBuiltinLoc, ToRParenLoc;
+  QualType ToType;
+  std::tie(ToCond, ToLHS, ToRHS, ToBuiltinLoc, ToRParenLoc, ToType) = *Imp;
+
+  ExprValueKind VK = E->getValueKind();
+  ExprObjectKind OK = E->getObjectKind();
+
+  bool TypeDependent = ToCond->isTypeDependent();
+  bool ValueDependent = ToCond->isValueDependent();
+
+  // The value of CondIsTrue only matters if the value is not
+  // condition-dependent.
+  bool CondIsTrue = false;
+  if (!E->isConditionDependent())
+CondIsTrue = E->i

[PATCH] D58292: Add support for importing ChooseExpr AST nodes.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder added inline comments.



Comment at: test/ASTMerge/choose-expr/Inputs/choose1.c:1
+#define abs_int(x) __builtin_choose_expr( \
+__builtin_types_compatible_p(__typeof(x), unsigned int),  \

a_sidorin wrote:
> This test duplicates unit test. I think we can keep unit test only and remove 
> this one.
> The other option (I like it even more) is to turn this test into constexpr 
> check and verify that this code _behaves_ as expected, not only that its AST 
> structure is fine. You can find some examples in ASTMerge tests - for expr 
> import, for example.
I can't use constexpr (and static_assert, as in the expr tests) because those 
are C++ keywords, and __builtin_choose_expr is only in C.

Instead, I used _Static_assert (from C11) to get a similar effect. Please take 
a look.



Comment at: unittests/AST/ASTImporterTest.cpp:1101
+  Decl *FromTU = getTuDecl(
+  R"(
+  #define abs_int(x) __builtin_choose_expr(\

a_sidorin wrote:
> I don't think we really need such macros in unit test just to check that the 
> expr is imported correctly - a single valid __builtin_choose_expr is enough. 
> It can even be checked with a simple `testImport()` call.
OK, dropped the test, since I already have that in ImportExpr.ImportChooseExpr 
in this patch.



Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:765
+  EXPECT_TRUE(matchesC(R"(
+  void f() {
+int x = -10;

a_sidorin wrote:
> Same here - I think the tests should be concise if it is possible.
OK, dropped this part of the test too, since the first part checks that a 
simple __builtin_choose_expr matches correctly.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58292



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

[PATCH] D58293: [clang][Index] Enable indexing of Template Type Parameters behind a flag

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM, but see the comment about avoiding code duplication




Comment at: lib/Index/IndexDecl.cpp:678
 IndexCtx.indexTypeSourceInfo(TTP->getDefaultArgumentInfo(), 
Parent);
+  if(IndexCtx.shouldIndexTemplateParameters())
+IndexCtx.handleDecl(TTP);

Maybe do it once on top of `TP` to avoid code duplication?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58293



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

[PATCH] D58388: [OpenCL] Simplify LLVM IR generated for OpenCL blocks

[Alexey Sotkin via Phabricator via cfe-commits]

AlexeySotkin updated this revision to Diff 187399.
AlexeySotkin added a comment.

Fix ObjC lit tests failure


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

https://reviews.llvm.org/D58388

Files:
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGOpenCLRuntime.cpp
  lib/CodeGen/CGOpenCLRuntime.h
  test/CodeGenOpenCL/blocks.cl
  test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Index: test/CodeGenOpenCL/cl20-device-side-enqueue.cl
===
--- test/CodeGenOpenCL/cl20-device-side-enqueue.cl
+++ test/CodeGenOpenCL/cl20-device-side-enqueue.cl
@@ -312,9 +312,7 @@
   };
 
   // Uses global block literal [[BLG8]] and invoke function [[INVG8]].
-  // COMMON: [[r1:%.*]] = load i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* getelementptr inbounds (%struct.__opencl_block_literal_generic, %struct.__opencl_block_literal_generic addrspace(4)* addrspacecast (%struct.__opencl_block_literal_generic addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BLG8]] to %struct.__opencl_block_literal_generic addrspace(1)*) to %struct.__opencl_block_literal_generic addrspace(4)*), i32 0, i32 2)
-  // COMMON: [[r2:%.*]] = addrspacecast i8 addrspace(4)* [[r1]] to void (i8 addrspace(4)*)*
-  // COMMON: call spir_func void [[r2]](i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BLG8]] to i8 addrspace(1)*) to i8 addrspace(4)*))
+  // COMMON: call spir_func void @__device_side_enqueue_block_invoke_11(i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BLG8]] to i8 addrspace(1)*) to i8 addrspace(4)*))
   block_A();
 
   // Emits global block literal [[BLG8]] and block kernel [[INVGK8]]. [[INVGK8]] calls [[INVG8]].
@@ -333,9 +331,7 @@
   unsigned size = get_kernel_work_group_size(block_A);
 
   // Uses global block literal [[BLG8]] and invoke function [[INVG8]]. Make sure no redundant block literal and invoke functions are emitted.
-  // COMMON: [[r1:%.*]] = load i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* getelementptr inbounds (%struct.__opencl_block_literal_generic, %struct.__opencl_block_literal_generic addrspace(4)* addrspacecast (%struct.__opencl_block_literal_generic addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BLG8]] to %struct.__opencl_block_literal_generic addrspace(1)*) to %struct.__opencl_block_literal_generic addrspace(4)*), i32 0, i32 2)
-  // COMMON: [[r2:%.*]] = addrspacecast i8 addrspace(4)* [[r1]] to void (i8 addrspace(4)*)*
-  // COMMON: call spir_func void [[r2]](i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BLG8]] to i8 addrspace(1)*) to i8 addrspace(4)*))
+  // COMMON: call spir_func void @__device_side_enqueue_block_invoke_11(i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BLG8]] to i8 addrspace(1)*) to i8 addrspace(4)*))
   block_A();
 
   void (^block_C)(void) = ^{
Index: test/CodeGenOpenCL/blocks.cl
===
--- test/CodeGenOpenCL/blocks.cl
+++ test/CodeGenOpenCL/blocks.cl
@@ -39,11 +39,8 @@
   // SPIR: %[[blk_gen_ptr:.*]] = addrspacecast %struct.__opencl_block_literal_generic* %[[blk_ptr]] to %struct.__opencl_block_literal_generic addrspace(4)*
   // SPIR: store %struct.__opencl_block_literal_generic addrspace(4)* %[[blk_gen_ptr]], %struct.__opencl_block_literal_generic addrspace(4)** %[[block_B:.*]],
   // SPIR: %[[block_literal:.*]] = load %struct.__opencl_block_literal_generic addrspace(4)*, %struct.__opencl_block_literal_generic addrspace(4)** %[[block_B]]
-  // SPIR: %[[invoke_addr:.*]] = getelementptr inbounds %struct.__opencl_block_literal_generic, %struct.__opencl_block_literal_generic addrspace(4)* %[[block_literal]], i32 0, i32 2
   // SPIR: %[[blk_gen_ptr:.*]] = bitcast %struct.__opencl_block_literal_generic addrspace(4)* %[[block_literal]] to i8 addrspace(4)*
-  // SPIR: %[[invoke_func_ptr:.*]] = load i8 addrspace(4)*, i8 addrspace(4)* addrspace(4)* %[[invoke_addr]]
-  // SPIR: %[[invoke_func:.*]] = addrspacecast i8 addrspace(4)* %[[invoke_func_ptr]] to i32 (i8 addrspace(4)*)*
-  // SPIR: call {{.*}}i32 %[[invoke_func]](i8 addrspace(4)* %[[blk_gen_ptr]])
+  // SPIR: call {{.*}}i32 @__foo_block_invoke(i8 addrspace(4)* %[[blk_gen_ptr]])
   // AMDGCN: %[[block_invoke:.*]] = getelementptr inbounds <{ i32, i32, i8*, i32 }>, <{ i32, i32, i8*, i32 }> addrspace(5)* %[[block:.*]], i32 0, i32 2
   // AMDGCN: store i8* bitcast (i32 (i8*)* @__foo_block_invoke to i8*), i8* addrspace(5)* %[[block_invoke]]
   // AMDGCN: %[[block_captured:.*]] = getelementptr inbounds <{ i32, i32, i8*, i32 }>, <{ i32, i32, i8*, i32 }> addrspace(5)* %[[block]], i32 0, i32 3
@@ -53,11 +50,8 @@
   // AMDGCN: %[[blk_gen_ptr:.*]] = addrspacecast %struct.__opencl_block_literal_generic addrspace(5)* %[[blk_ptr]] to %struct.__opencl_block_literal_g

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[pierre gousseau via Phabricator via cfe-commits]

pgousseau updated this revision to Diff 187400.
pgousseau added a comment.

Updated to use an array as suggested.


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

https://reviews.llvm.org/D57914

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/Sanitizers.def
  include/clang/Basic/Sanitizers.h
  include/clang/Driver/ToolChain.h
  lib/Basic/SanitizerSpecialCaseList.cpp
  lib/Basic/Sanitizers.cpp
  lib/CodeGen/CGExpr.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclAttr.cpp

Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6199,7 +6199,8 @@
 if (!S.checkStringLiteralArgumentAttr(AL, I, SanitizerName, &LiteralLoc))
   return;
 
-if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) == 0)
+if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) ==
+SanitizerMask())
   S.Diag(LiteralLoc, diag::warn_unknown_sanitizer_ignored) << SanitizerName;
 else if (isGlobalVar(D) && SanitizerName != "address")
   S.Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -551,7 +551,7 @@
 DiagnosticsEngine &Diags, SanitizerSet &S) {
   for (const auto &Sanitizer : Sanitizers) {
 SanitizerMask K = parseSanitizerValue(Sanitizer, /*AllowGroups=*/false);
-if (K == 0)
+if (K == SanitizerMask())
   Diags.Report(diag::err_drv_invalid_value) << FlagName << Sanitizer;
 else
   S.set(K, true);
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -25,29 +25,32 @@
 using namespace clang::driver;
 using namespace llvm::opt;
 
-enum : SanitizerMask {
-  NeedsUbsanRt = Undefined | Integer | ImplicitConversion | Nullability | CFI,
-  NeedsUbsanCxxRt = Vptr | CFI,
-  NotAllowedWithTrap = Vptr,
-  NotAllowedWithMinimalRuntime = Vptr,
-  RequiresPIE = DataFlow | HWAddress | Scudo,
-  NeedsUnwindTables = Address | HWAddress | Thread | Memory | DataFlow,
-  SupportsCoverage = Address | HWAddress | KernelAddress | KernelHWAddress |
- Memory | KernelMemory | Leak | Undefined | Integer |
- ImplicitConversion | Nullability | DataFlow | Fuzzer |
- FuzzerNoLink,
-  RecoverableByDefault = Undefined | Integer | ImplicitConversion | Nullability,
-  Unrecoverable = Unreachable | Return,
-  AlwaysRecoverable = KernelAddress | KernelHWAddress,
-  LegacyFsanitizeRecoverMask = Undefined | Integer,
-  NeedsLTO = CFI,
-  TrappingSupported = (Undefined & ~Vptr) | UnsignedIntegerOverflow |
-  ImplicitConversion | Nullability | LocalBounds | CFI,
-  TrappingDefault = CFI,
-  CFIClasses =
-  CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
-};
+SanitizerMask NeedsUbsanRt =
+Undefined | Integer | ImplicitConversion | Nullability | CFI;
+SanitizerMask NeedsUbsanCxxRt = Vptr | CFI;
+SanitizerMask NotAllowedWithTrap = Vptr;
+SanitizerMask NotAllowedWithMinimalRuntime = Vptr;
+SanitizerMask RequiresPIE = DataFlow | HWAddress | Scudo;
+SanitizerMask NeedsUnwindTables =
+Address | HWAddress | Thread | Memory | DataFlow;
+SanitizerMask SupportsCoverage =
+Address | HWAddress | KernelAddress | KernelHWAddress | Memory |
+KernelMemory | Leak | Undefined | Integer | ImplicitConversion |
+Nullability | DataFlow | Fuzzer | FuzzerNoLink;
+SanitizerMask RecoverableByDefault =
+Undefined | Integer | ImplicitConversion | Nullability;
+SanitizerMask Unrecoverable = Unreachable | Return;
+SanitizerMask AlwaysRecoverable = KernelAddress | KernelHWAddress;
+SanitizerMask LegacyFsanitizeRecoverMask = Undefined | Integer;
+SanitizerMask NeedsLTO = CFI;
+SanitizerMask TrappingSupported = (Undefined & ~Vptr) |
+  UnsignedIntegerOverflow | ImplicitConversion |
+  Nullability | LocalBounds | CFI;
+SanitizerMask TrappingDefault = CFI;
+SanitizerMask CFIClasses =
+CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast;
+SanitizerMask CompatibleWithMinimalRuntime =
+TrappingSupported | Scudo | ShadowCallStack;
 
 enum CoverageFeature {
   CoverageFunc = 1 << 0,
@@ -136,10 +139,10 @@
 
 static SanitizerMask parseSanitizeTrapArgs(const Driver &D,
const llvm::opt::ArgList &Args) {
-  SanitizerMask TrapRemove = 0; // During the loop below, the accumulated set of
+  SanitizerMask TrapRemove; // During the loop below, the accumulated set of
  

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Basic/Targets/RISCV.h:94
+if (HasA)
+  MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 32;
+  }

MaxAtomicPromoteWidth is an incompatible ABI-change kind of thing.

We should set that to the maximum atomic size this target ABI will _EVER_ 
support with any hardware, since it changes the data layout for atomic types.

MaxAtomicInlineWidth can change based on hardware, and be increased in the 
future if other hardware is introduced, but MaxAtomicPromoteWidth shouldn't.

I think it should be set it to 64 for most 32-bit platforms, and 128 for most 
64-bit platforms.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57450



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

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.

There is a long series of comments in this patch and I am not clear at this 
point on whether this patch breaks anything or it is fine. Could you please 
`Request Changes` if this patch is broken or approve if it is fine?


Repository:
  rC Clang

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

https://reviews.llvm.org/D49754



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

Re: r354075 - [clang][FileManager] fillRealPathName even if we aren't opening the file

[Jan Korous via cfe-commits]

I see. You're right the name sounds slow indeed. Thank you for the explanation!

> On Feb 19, 2019, at 6:43 AM, Nico Weber  wrote:
> 
> I didn't realize it just copied a string. I just saw a call to 
> "fillRealPathName" and the "RealPath" bit sounded slow. From clicking around 
> in http://llvm-cs.pcc.me.uk/tools/clang/lib/Basic/FileManager.cpp#361 
>  it looks 
> like it's not really computing a realpath (for symlinks). It still does quite 
> a bit of string processing if InterndFileName isn't absolute, but that's 
> probably fine. It's still the kind of thing I'd carefully measure since 
> getFile(openFile=false) was performance-sensitive in a certain case iirc (I 
> think when using pch files?)
> 
> On Mon, Feb 18, 2019 at 5:26 PM Jan Korous  > wrote:
> Hi Nico,
> 
> I didn't think it necessary as the change doesn't introduce any interaction 
> with filesystem - it's just copying a string.
> 
> Do you mean it causes a performance regression?
> 
> Thanks.
> 
> Jan
> 
>> On Feb 15, 2019, at 6:15 AM, Nico Weber > > wrote:
>> 
>> Did you do any performance testing to check if this slows down clang?
>> 
>> On Thu, Feb 14, 2019 at 6:02 PM Jan Korous via cfe-commits 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> Author: jkorous
>> Date: Thu Feb 14 15:02:35 2019
>> New Revision: 354075
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=354075&view=rev 
>> 
>> Log:
>> [clang][FileManager] fillRealPathName even if we aren't opening the file
>> 
>> The pathname wasn't previously filled when the getFile() method was called 
>> with openFile = false.
>> We are caching FileEntry-s in ParsedAST::Includes in clangd and this caused 
>> the problem.
>> 
>> This fixes an internal test failure in clangd - ClangdTests.GoToInclude.All
>> 
>> rdar://47536127 <>
>> 
>> Differential Revision: https://reviews.llvm.org/D58213 
>> 
>> 
>> Modified:
>> cfe/trunk/lib/Basic/FileManager.cpp
>> cfe/trunk/unittests/Basic/FileManagerTest.cpp
>> 
>> Modified: cfe/trunk/lib/Basic/FileManager.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=354075&r1=354074&r2=354075&view=diff
>>  
>> 
>> ==
>> --- cfe/trunk/lib/Basic/FileManager.cpp (original)
>> +++ cfe/trunk/lib/Basic/FileManager.cpp Thu Feb 14 15:02:35 2019
>> @@ -267,6 +267,9 @@ const FileEntry *FileManager::getFile(St
>>if (UFE.File) {
>>  if (auto PathName = UFE.File->getName())
>>fillRealPathName(&UFE, *PathName);
>> +  } else if (!openFile) {
>> +// We should still fill the path even if we aren't opening the file.
>> +fillRealPathName(&UFE, InterndFileName);
>>}
>>return &UFE;
>>  }
>> 
>> Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=354075&r1=354074&r2=354075&view=diff
>>  
>> 
>> ==
>> --- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original)
>> +++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Thu Feb 14 15:02:35 2019
>> @@ -346,4 +346,18 @@ TEST_F(FileManagerTest, getVirtualFileFi
>>EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
>>  }
>> 
>> +TEST_F(FileManagerTest, getFileDontOpenRealPath) {
>> +  auto statCache = llvm::make_unique();
>> +  statCache->InjectDirectory("/tmp/abc", 42);
>> +  SmallString<64> Path("/tmp/abc/foo.cpp");
>> +  statCache->InjectFile(Path.str().str().c_str(), 43);
>> +  manager.setStatCache(std::move(statCache));
>> +
>> +  const FileEntry *file = manager.getFile(Path, /*openFile=*/false);
>> +
>> +  ASSERT_TRUE(file != nullptr);
>> +
>> +  ASSERT_EQ(file->tryGetRealPathName(), Path);
>> +}
>> +
>>  } // anonymous namespace
>> 
>> 
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org 
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits 
>> 
> 

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

[PATCH] D58243: [OPENMP] Delay emission of the asm target-specific error messages.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: lib/Sema/SemaStmtAsm.cpp:256-263
   // If we're compiling CUDA file and function attributes indicate that it's 
not
   // for this compilation side, skip all the checks.
   if (!DeclAttrsMatchCUDAMode(getLangOpts(), getCurFunctionDecl())) {
 GCCAsmStmt *NS = new (Context) GCCAsmStmt(
 Context, AsmLoc, IsSimple, IsVolatile, NumOutputs, NumInputs, Names,
 Constraints, Exprs.data(), AsmString, NumClobbers, Clobbers, 
RParenLoc);
 return NS;

Now that inline asm errors are delayed, do we still need this check?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58243



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

[PATCH] D58392: [analyzer] MIGChecker: Fix false negatives for releases in automatic destructors.

[Artem Dergachev via Phabricator via cfe-commits]

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

The problem that was fixed in D25326  for 
inlined functions keeps biting us for the top-level functions as well. Namely, 
i had to use `check::PreStmt` rather than `check::EndFunction` in 
D57558  because in the `basic_test` test the 
paths get merged before invoking the `check::EndFunction` callback. I don't 
have an immediate fix for the whole overall problem, so for now, as a hack, 
`check::PreStmt` keeps being used.

However, the problem with `check::PreStmt` is that automatic 
destructors for function-local variables are not yet evaluated when this 
callback is invoked. This causes false negatives in MIGChecker: if memory is 
released in an automatic destructor and then an error code is returned, the bug 
is not found because the Static Analyzer evalues the destructor *after* the 
pre-return. Arguably, this may also be treated as a bug, depending on what does 
the "Pre" in `PreStmt` stand for.

But regardless, for now it seems that we'd have to subscribe on both callbacks.


Repository:
  rC Clang

https://reviews.llvm.org/D58392

Files:
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/test/Analysis/mig.cpp


Index: clang/test/Analysis/mig.cpp
===
--- clang/test/Analysis/mig.cpp
+++ clang/test/Analysis/mig.cpp
@@ -44,3 +44,28 @@
   return ret; // expected-warning{{MIG callback fails with error after 
deallocating argument value. This is use-after-free vulnerability because 
caller will try to deallocate it again}}
  // expected-note@-1{{MIG callback fails with error after 
deallocating argument value. This is use-after-free vulnerability because 
caller will try to deallocate it again}}
 }
+
+// Make sure we find the bug when the object is destroyed within an
+// automatic destructor.
+MIG_SERVER_ROUTINE
+kern_return_t test_vm_deallocate_in_automatic_dtor(mach_port_name_t port, 
vm_address_t address, vm_size_t size) {
+  struct WillDeallocate {
+mach_port_name_t port;
+vm_address_t address;
+vm_size_t size;
+~WillDeallocate() {
+  vm_deallocate(port, address, size); // expected-note{{Deallocating 
object passed through parameter 'address'}}
+}
+  } will_deallocate{port, address, size};
+
+ if (size > 10) {
+// expected-note@-1{{Assuming 'size' is > 10}}
+// expected-note@-2{{Taking true branch}}
+return KERN_ERROR;
+// expected-note@-1{{Calling '~WillDeallocate'}}
+// expected-note@-2{{Returning from '~WillDeallocate'}}
+// expected-warning@-3{{MIG callback fails with error after deallocating 
argument value. This is use-after-free vulnerability because caller will try to 
deallocate it again}}
+// expected-note@-4   {{MIG callback fails with error after deallocating 
argument value. This is use-after-free vulnerability because caller will try to 
deallocate it again}}
+  }
+  return KERN_SUCCESS;
+}
Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
@@ -31,15 +31,30 @@
 using namespace ento;
 
 namespace {
-class MIGChecker : public Checker> 
{
+class MIGChecker : public Checker,
+  check::EndFunction> {
   BugType BT{this, "Use-after-free (MIG calling convention violation)",
  categories::MemoryError};
 
   CallDescription vm_deallocate { "vm_deallocate", 3 };
 
+  void checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const;
+
 public:
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
-  void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const;
+
+  // HACK: We're making two attempts to find the bug: checkEndFunction
+  // should normally be enough but it fails when the return value is a literal
+  // that never gets put into the Environment and ends of function with 
multiple
+  // returns get agglutinated across returns, preventing us from obtaining
+  // the return value. The problem is similar to 
https://reviews.llvm.org/D25326
+  // but now we step into it in the top-level function.
+  void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const {
+checkReturnAux(RS, C);
+  }
+  void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const {
+checkReturnAux(RS, C);
+  }
 };
 } // end anonymous namespace
 
@@ -103,7 +118,7 @@
   C.addTransition(C.getState()->set(true), T);
 }
 
-void MIGChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const {
+void MIGChecker::checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const 
{
   // It is very unlikely that a MIG callback will

[PATCH] D56411: [CUDA][HIP][Sema] Fix template kernel with function as template parameter

[Artem Belevich via Phabricator via cfe-commits]

tra added a subscriber: rsmith.
tra added a comment.

In D56411#1400300 , @rjmccall wrote:

> Okay, but it's not great design to have a kind of overloading that can't be 
> resolved to an exact intended declaration even by an explicit cast.  That's 
> why I think making *optional* host/device typing is a good idea.  And I 
> strongly want to caution you against doing language design by just 
> incrementally hacking at the compiler to progressively make more test-cases 
> work, which is what it feels like you're doing.


+1. IMO for templates to work sensibly in this situations `__host__` / 
`__device__` must be part of the type.

I.e. extending the example above,

  __host__ int f() { return 1;}
  __device__ int f() { return 2;}
  template __kernel__ void t() { F(); }
  __host__ void g() { t<<<1,1>>>(); }
  __global__ void g() { t<<<1,1>>>(); } // technically legal in CUDA, though 
clang does not support it yet.

IMO, t in `__host__` g() should be different from t in `__device__` g(). 
Which implies that 'device-ness' must be part of the F's type so we would have 
two different instantiations, which is what we want to see in the AST.
Calling context if somewhat irrelevant for template instantiations. E.g. one 
could've explicitly instantiated the template in the global scope.

@rsmith Any suggestions how we could deal with this situation in a principled 
way?


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

https://reviews.llvm.org/D56411



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

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang updated this revision to Diff 187418.

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

https://reviews.llvm.org/D50488

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
  test/Analysis/ptr-sort.cpp

Index: test/Analysis/ptr-sort.cpp
===
--- /dev/null
+++ test/Analysis/ptr-sort.cpp
@@ -0,0 +1,57 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.nondeterminism.PointerSorting %s -analyzer-output=text -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+namespace std {
+  template
+  bool is_sorted(ForwardIt first, ForwardIt last);
+
+  template 
+  void nth_element(RandomIt first, RandomIt nth, RandomIt last);
+
+  template
+  void partial_sort(RandomIt first, RandomIt middle, RandomIt last);
+
+  template
+  void sort (RandomIt first, RandomIt last);
+
+  template
+  void stable_sort(RandomIt first, RandomIt last);
+
+  template
+  BidirIt partition(BidirIt first, BidirIt last, UnaryPredicate p);
+
+  template
+  BidirIt stable_partition(BidirIt first, BidirIt last, UnaryPredicate p);
+}
+
+bool f (int x) { return true; }
+bool g (int *x) { return true; }
+
+void PointerSorting() {
+  int a = 1, b = 2, c = 3;
+  std::vector V1 = {a, b};
+  std::vector V2 = {&a, &b};
+
+  std::is_sorted(V1.begin(), V1.end());// no-warning
+  std::nth_element(V1.begin(), V1.begin() + 1, V1.end());  // no-warning
+  std::partial_sort(V1.begin(), V1.begin() + 1, V1.end()); // no-warning
+  std::sort(V1.begin(), V1.end()); // no-warning
+  std::stable_sort(V1.begin(), V1.end());  // no-warning
+  std::partition(V1.begin(), V1.end(), f); // no-warning
+  std::stable_partition(V1.begin(), V1.end(), g);  // no-warning
+
+  std::is_sorted(V2.begin(), V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  std::nth_element(V2.begin(), V2.begin() + 1, V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  std::partial_sort(V2.begin(), V2.begin() + 1, V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  std::sort(V2.begin(), V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  std::stable_sort(V2.begin(), V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  std::partition(V2.begin(), V2.end(), f); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  std::stable_partition(V2.begin(), V2.end(), g); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+}
Index: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
@@ -0,0 +1,117 @@
+//===-- PointerSortingChecker.cpp -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file defines PointerSortingChecker which checks for non-determinism
+// caused due to sorting containers with pointer-like elements.
+//
+//===--===//
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#includ

[PATCH] D58397: [analyzer] MIGChecker: Pour more data into the checker.

[Artem Dergachev via Phabricator via cfe-commits]

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

Previous patches were about the infrastructure of the checker. Right now the 
infrastructure is pretty much complete, so let's make use. Namely:

- Add more release functions. For now only `vm_deallocate()` was supported. 
I'll also have a look at adding an attribute so that users could annotate their 
own releasing functions, but hardcoding a few popular APIs wouldn't hurt.
- Add a non-zero value that isn't an error; this value is -315 ("MIG_NO_REPLY") 
and it's fine to deallocate data when you are returning this error.
- Make sure that the `mig_server_routine` annotation is inherited. Mmm, not 
sure, i expected Sema to do this automatically. I'll ask in D58365 
.


Repository:
  rC Clang

https://reviews.llvm.org/D58397

Files:
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/test/Analysis/mig.cpp

Index: clang/test/Analysis/mig.cpp
===
--- clang/test/Analysis/mig.cpp
+++ clang/test/Analysis/mig.cpp
@@ -1,21 +1,60 @@
 // RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.osx.MIG\
 // RUN:   -analyzer-output=text -verify %s
 
+typedef unsigned uint32_t;
 
 // XNU APIs.
 
 typedef int kern_return_t;
 #define KERN_SUCCESS 0
 #define KERN_ERROR 1
+#define MIG_NO_REPLY (-305)
 
 typedef unsigned mach_port_name_t;
 typedef unsigned vm_address_t;
 typedef unsigned vm_size_t;
+typedef void *ipc_space_t;
+typedef unsigned long io_user_reference_t;
 
 kern_return_t vm_deallocate(mach_port_name_t, vm_address_t, vm_size_t);
 
 #define MIG_SERVER_ROUTINE __attribute__((mig_server_routine))
 
+ kern_return_t mach_vm_deallocate(mach_port_name_t, vm_address_t, vm_size_t);
+ void mig_deallocate(vm_address_t, vm_size_t);
+ kern_return_t mach_port_deallocate(ipc_space_t, mach_port_name_t);
+
+
+// IOKit wrappers.
+
+class OSObject;
+typedef kern_return_t IOReturn;
+#define kIOReturnError 1
+
+enum {
+  kOSAsyncRef64Count = 8,
+};
+
+typedef io_user_reference_t OSAsyncReference64[kOSAsyncRef64Count];
+
+struct IOExternalMethodArguments {
+  io_user_reference_t *asyncReference;
+};
+
+struct IOExternalMethodDispatch {};
+
+class IOUserClient {
+public:
+  static IOReturn releaseAsyncReference64(OSAsyncReference64);
+
+  MIG_SERVER_ROUTINE
+  virtual IOReturn externalMethod(uint32_t selector, IOExternalMethodArguments *arguments,
+  IOExternalMethodDispatch *dispatch = 0, OSObject *target = 0, void *reference = 0);
+};
+
+
+// Tests.
+
 MIG_SERVER_ROUTINE
 kern_return_t basic_test(mach_port_name_t port, vm_address_t address, vm_size_t size) {
   vm_deallocate(port, address, size); // expected-note{{Deallocating object passed through parameter 'address'}}
@@ -69,3 +108,51 @@
   }
   return KERN_SUCCESS;
 }
+
+// Test various APIs.
+MIG_SERVER_ROUTINE
+kern_return_t test_mach_vm_deallocate(mach_port_name_t port, vm_address_t address, vm_size_t size) {
+  mach_vm_deallocate(port, address, size); // expected-note{{Deallocating object passed through parameter 'address'}}
+  return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value}}
+ // expected-note@-1{{MIG callback fails with error after deallocating argument value}}
+}
+
+MIG_SERVER_ROUTINE
+kern_return_t test_mach_port_deallocate(ipc_space_t space,
+mach_port_name_t port) {
+  mach_port_deallocate(space, port); // expected-note{{Deallocating object passed through parameter 'port'}}
+  return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value}}
+ // expected-note@-1{{MIG callback fails with error after deallocating argument value}}
+}
+
+MIG_SERVER_ROUTINE
+kern_return_t test_mig_deallocate(vm_address_t address, vm_size_t size) {
+  mig_deallocate(address, size); // expected-note{{Deallocating object passed through parameter 'address'}}
+  return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value}}
+ // expected-note@-1{{MIG callback fails with error after deallocating argument value}}
+}
+
+MIG_SERVER_ROUTINE
+IOReturn test_releaseAsyncReference64(IOExternalMethodArguments *arguments) {
+  IOUserClient::releaseAsyncReference64(arguments->asyncReference); // expected-note{{Deallocating object passed through parameter 'arguments'}}
+  return kIOReturnError;// expected-warning{{MIG callback fails with error after deallocating argument value}}
+// expec

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

In D50488#1400823 , @whisperity wrote:

> Yeah, it seems upstream has moved away due to @Szelethus' implementation of a 
> much more manageable "checker dependency" system. You most likely will have 
> to rebase your patch first, then check what you missed which got added to 
> other merged, existing checkers.


Yes, I see that D55429 , D54438 
 added a new dependency field for checkers. 
However, I see that not all checkers need/have that field. In this case, 
shouldn't providing a ento::shouldRegisterPointerSortingChecker function which 
returns true be enough to register the checker?
Moreover, as I said I hit this assert only when invoking the checker via 
csa-testbench. Outside that it seems to work fine. For example, my unit test 
Analysis/ptr-sort.cpp passes.


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

https://reviews.llvm.org/D50488



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

[PATCH] D58365: [attributes] Add a MIG server routine attribute.

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/AttrDocs.td:4047-4048
+that have their return value of type ``kern_return_t`` unconditionally returned
+from the routine. The attribute can be applied to C++ methods, and in this case
+it will be automatically applied to overrides if the method is virtual.
+}];

Is there a way to enforce this automagically via adding some feature to the 
attribute? Or should i enforce it manually by teaching Static Analyzer to scan 
the overridden methods when looking for the attribute, like i did in D58397?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58365



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

[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

[Zinovy Nis via Phabricator via cfe-commits]

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

- `-j` is `1` by default;
- fixed minor remarks;


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

https://reviews.llvm.org/D57662

Files:
  clang-tidy/tool/clang-tidy-diff.py

Index: clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tidy/tool/clang-tidy-diff.py
+++ clang-tidy/tool/clang-tidy-diff.py
@@ -25,9 +25,56 @@
 
 import argparse
 import json
+import multiprocessing
+import os
 import re
 import subprocess
 import sys
+import threading
+
+is_py2 = sys.version[0] == '2'
+
+if is_py2:
+import Queue as queue
+else:
+import queue as queue
+
+def run_tidy(task_queue, lock, timeout):
+  watchdog = None
+  while True:
+command = task_queue.get()
+try:
+  proc = subprocess.Popen(command,
+  stdout=subprocess.PIPE,
+  stderr=subprocess.PIPE)
+
+  if timeout is not None:
+watchdog = threading.Timer(timeout, proc.kill)
+watchdog.start()
+
+  stdout, stderr = proc.communicate()
+
+  with lock:
+sys.stdout.write((' '.join(command)).decode('utf-8') + '\n' + stdout.decode('utf-8') + '\n')
+if stderr:
+  sys.stderr.write(stderr.decode('utf-8') + '\n')
+except Exception as e:
+  with lock:
+sys.stderr.write('Failed: ' + str(e) + ' '.join(command) + '\n')
+finally:
+  with lock:
+if (not timeout is None) and (not watchdog is None):
+  if not watchdog.is_alive():
+  sys.stderr.write('Terminated by timeout: ' + ' '.join(command) + '\n')
+  watchdog.cancel()
+  task_queue.task_done()
+
+
+def run_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
+  for _ in range(max_tasks):
+t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
+t.daemon = True
+t.start()
 
 
 def main():
@@ -48,6 +95,10 @@
   help='custom pattern selecting file paths to check '
   '(case insensitive, overridden by -regex)')
 
+  parser.add_argument('-j', type=int, default=1,
+  help='number of tidy instances to be run in parallel.')
+  parser.add_argument('-timeout', type=int, default=None,
+  help='timeout per each file in seconds.')
   parser.add_argument('-fix', action='store_true', default=False,
   help='apply suggested fixes')
   parser.add_argument('-checks',
@@ -77,6 +128,11 @@
 
   args = parser.parse_args(argv)
 
+  if args.j == 0 or args.j > 1:
+if args.export_fixes:
+  print("error: -export-fixes and -j are mutually exclusive.")
+  sys.exit(1)
+
   # Extract changed lines for each file.
   filename = None
   lines_by_file = {}
@@ -84,7 +140,7 @@
 match = re.search('^\+\+\+\ \"?(.*?/){%s}([^ \t\n\"]*)' % args.p, line)
 if match:
   filename = match.group(2)
-if filename == None:
+if filename is None:
   continue
 
 if args.regex is not None:
@@ -102,44 +158,64 @@
 line_count = int(match.group(3))
   if line_count == 0:
 continue
-  end_line = start_line + line_count - 1;
+  end_line = start_line + line_count - 1
   lines_by_file.setdefault(filename, []).append([start_line, end_line])
 
-  if len(lines_by_file) == 0:
+  if not any(lines_by_file):
 print("No relevant changes found.")
 sys.exit(0)
 
-  line_filter_json = json.dumps(
-[{"name" : name, "lines" : lines_by_file[name]} for name in lines_by_file],
-separators = (',', ':'))
+  max_task = args.j
+  if max_task == 0:
+  max_task = multiprocessing.cpu_count()
+  max_task = min(len(lines_by_file), max_task)
+
+  # Tasks for clang-tidy.
+  task_queue = queue.Queue(max_task)
+  # A lock for console output.
+  lock = threading.Lock()
 
-  quote = "";
-  if sys.platform == 'win32':
-line_filter_json=re.sub(r'"', r'"""', line_filter_json)
-  else:
-quote = "'";
+  # Run a pool of clang-tidy workers.
+  run_workers(max_task, run_tidy, task_queue, lock, args.timeout)
 
-  # Run clang-tidy on files containing changes.
-  command = [args.clang_tidy_binary]
-  command.append('-line-filter=' + quote + line_filter_json + quote)
+  quote = ""
+  if sys.platform != 'win32':
+quote = "'"
+
+  # Form the common args list.
+  common_clang_tidy_args = []
   if args.fix:
-command.append('-fix')
+common_clang_tidy_args.append('-fix')
   if args.export_fixes:
-command.append('-export-fixes=' + args.export_fixes)
+common_clang_tidy_args.append('-export-fixes=' + args.export_fixes)
   if args.checks != '':
-command.append('-checks=' + quote + args.checks + quote)
+common_clang_tidy_args.append('-checks=' + quote + args.checks + quote)
   if args.quiet:
-command.append('-quiet')
+common_clang_tidy_args.append('-quiet')
   if args.build_path is not None:
-command.append('-p=%s' % args.buil

[PATCH] D57977: [HIP] compile option code-object-v3 propagate to llc

[Aaron Enye Shi via Phabricator via cfe-commits]

ashi1 added a comment.

This was submitted again with the fix. Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57977



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

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[Bruno Ricci via Phabricator via cfe-commits]

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

Looks mostly fine to me. I have added a few inline comments but they are all 
little nits.




Comment at: include/clang/Basic/Sanitizers.h:39
+struct SanitizerMask {
+private:
+  // Number of array elements.

`struct` -> `class` and get rid of the `private` ?



Comment at: include/clang/Basic/Sanitizers.h:46
+  static constexpr unsigned kNumBits = sizeof(maskLoToHigh) * 8;
+  // Number of bits in a mask element.
+  static constexpr unsigned kNumBitElem = sizeof(maskLoToHigh[0]) * 8;

/// instead to have doxygen handle these comments (here and elsewhere).



Comment at: include/clang/Basic/Sanitizers.h:51
+  // Mask is initialized to 0.
+  SanitizerMask() : maskLoToHigh{} {}
+

You don't have to explicitly write the default ctor; you can just instead write 
`uint64_t maskLoToHigh[kNumElem]{};` above to value-initialize each element in 
the array.



Comment at: include/clang/Basic/Sanitizers.h:54
+  static constexpr bool
+  checkBitPos(const SanitizerKind::SanitizerOrdinal &Pos) {
+return Pos < kNumBits;

`SanitizerOrdinal` should probably be passed by value here and elsewhere.



Comment at: include/clang/Basic/Sanitizers.h:61
+  bitPosToMask(const SanitizerKind::SanitizerOrdinal &Pos) {
+assert(Pos < kNumBits);
+SanitizerMask mask;

Could you please add a relevant message to the assertion (here and elsewhere) ?



Comment at: include/clang/Basic/Sanitizers.h:96
+return false;
+}
+return true;

Is that vectorized by gcc/clang, or is that a sequence of branches ?



Comment at: lib/Driver/SanitizerArgs.cpp:53
+SanitizerMask CompatibleWithMinimalRuntime =
+TrappingSupported | Scudo | ShadowCallStack;
 

Shouldn't all of these masks be const to be sure that they are not modified by 
mistake (unless I a missing something and they are modified somewhere else) ?


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

https://reviews.llvm.org/D57914



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

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[vit9696 via Phabricator via cfe-commits]

vit9696 added a comment.

This is a series of patches, which I believe should merged altogether. 
Currently the following patches are relevant:

- https://reviews.llvm.org/D49754
- https://reviews.llvm.org/D54409
- https://reviews.llvm.org/D54583
- https://reviews.llvm.org/D56703

The patches are intended to add PowerPC SPE support, and they do not seem to 
break things outside. Initially I wanted them to get merged into 8.x, but 
extensive local testing unveiled a number of issues, so the time was missed. I 
believe all the 4 patches are pretty much ready for merging, aside the 
following:

- approval from @jhibbits and @kthomsen, who do some testing as well
- https://reviews.llvm.org/D49754#1401168 is fixed or declared a separate 
issue, handled outside of the patchset

Depending on the above the decision should be taken. Meanwhile, a test should 
be added for `__NO_FPRS__` near `PPC32-SPE:#define __SPE__ 1 ` now that we 
define it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49754



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

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

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

Szelethus added a comment.

I love the idea! It looks way cleaner, tbh how messy visitors are have kept me 
away from implementing one for my own checker. Couple thoughts:

1. It would be great to have unit tests for this. Side note, I have already 
managed to make CSA unit tests tun under check-clang-analysis, but it makes 
check-clang and check-all run them twice, either way, if you find it helpful, I 
have a branch for it here: * I need to get home where I can push it to github, 
imagine a nice link here *. That should make development a tad bit less painful 
:)
2. I guess most of the visitors could be changed to this format, do you have 
plans to convert them? I'll happily land a hand, because it sounds like a big 
chore. I guess that would also test this implementation fairly well.
3. Either in your workbook, or even better, in the new sphinx documentation, it 
would be great to see a how-to. Although I guess it's fine to leave some time 
for this to mature.

Go at it, this really is great! ^-^


Repository:
  rC Clang

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

https://reviews.llvm.org/D58367



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

[PATCH] D58243: [OPENMP] Delay emission of the asm target-specific error messages.

[Alexey Bataev via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaStmtAsm.cpp:256-263
   // If we're compiling CUDA file and function attributes indicate that it's 
not
   // for this compilation side, skip all the checks.
   if (!DeclAttrsMatchCUDAMode(getLangOpts(), getCurFunctionDecl())) {
 GCCAsmStmt *NS = new (Context) GCCAsmStmt(
 Context, AsmLoc, IsSimple, IsVolatile, NumOutputs, NumInputs, Names,
 Constraints, Exprs.data(), AsmString, NumClobbers, Clobbers, 
RParenLoc);
 return NS;

tra wrote:
> Now that inline asm errors are delayed, do we still need this check?
With this patch, it is going to be delayed only for OpenMP. After the commit, 
we could extend it for CUDA and remove this code. But not at the moment.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58243



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

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

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

Szelethus added a comment.

In D50488#1402699 , @mgrang wrote:

> In D50488#1400823 , @whisperity 
> wrote:
>
> > Yeah, it seems upstream has moved away due to @Szelethus' implementation of 
> > a much more manageable "checker dependency" system. You most likely will 
> > have to rebase your patch first, then check what you missed which got added 
> > to other merged, existing checkers.
>
>
> Moreover, as I said I hit this assert only when invoking the checker via 
> csa-testbench. Outside that it seems to work fine. For example, my unit test 
> Analysis/ptr-sort.cpp passes.


It's because it invokes CodeChecker, which by default enables 
`valist.Uninitialized`, but not `ValistBase`. Do you have assert failures after 
my hotfix?

> Yes, I see that D55429 , D54438 
>  added a new dependency field for checkers. 
> However, I see that not all checkers need/have that field. In this case, 
> shouldn't providing a ento::shouldRegisterPointerSortingChecker function 
> which returns true be enough to register the checker?

It is enough, and you're very right about this not being properly documented! 
There is a revision to solve this problem here: D58065 
. I guess your input, as someone who didn't 
participate in the checker dependency related patch reviews would be 
invaluable, in terms of whether my description is understandable enough.


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

https://reviews.llvm.org/D50488



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

[PATCH] D58365: [attributes] Add a MIG server routine attribute.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

> @aaron.ballman, are you actually interested in reviewing these attributes? 
> 'Cause George added you on his reviews but i totally understand that these 
> are fairly exotic.

Yeah, I'd like to review these attributes -- thanks for double-checking with me!




Comment at: clang/include/clang/Basic/Attr.td:1310
 
+def MIGServerRoutine : InheritableAttr {
+  let Spellings = [Clang<"mig_server_routine">];

Should this be limited to specific targets, or is this a general concept that 
applies to all targets?



Comment at: clang/include/clang/Basic/Attr.td:1312
+  let Spellings = [Clang<"mig_server_routine">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [MIGCallingConventionDocs];

Objective-C methods as well?



Comment at: clang/include/clang/Basic/Attr.td:1313
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [MIGCallingConventionDocs];
+}

This isn't really a calling convention though, is it? It doesn't change 
codegen, impact function types, or anything like that.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8702
+def warn_mig_server_routine_does_not_return_kern_return_t : Warning<
+  "'%0' attribute only applies to functions that return a kernel return code">,
+  InGroup;

Will users understand "kernel return code"? Should this say `kern_return_t` 
explicitly?

No need to use %0 here, just spell out the attribute name directly (unless you 
expect this to be used by multiple attributes, in which case the name of the 
diagnostic should be changed).



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7142
+
+  // Mach Interface Generator annotations.
+  case ParsedAttr::AT_MIGServerRoutine:

This comment doesn't add a ton of value, I'd probably remove it.



Comment at: clang/test/Sema/attr-mig.c:6
+
+__attribute__((mig_server_routine)) kern_return_t var = KERN_SUCCESS; // 
expected-warning-re{{'mig_server_routine' attribute only applies to 
functions{{$
+

Can you also add a test showing that the attribute doesn't accept arguments.



Comment at: clang/test/Sema/attr-mig.c:17
+}
+
+kern_return_t bar_forward() { // no-warning

Here's an edge case to consider:
```
__attribute__((mig_server_routine)) int foo(void);

typedef int kern_return_t;

kern_return_t foo(void) { ... }
```
Do you have to care about situations like that?



Comment at: clang/test/Sema/attr-mig.cpp:10
+public:
+  virtual __attribute__((mig_server_routine)) IOReturn externalMethod();
+  virtual __attribute__((mig_server_routine)) void anotherMethod(); // 
expected-warning{{'mig_server_routine' attribute only applies to functions that 
return a kernel return code}}

Can you use the C++ spelling for the attribute, so we have a bit of coverage 
for that?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58365



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

[PATCH] D58243: [OPENMP] Delay emission of the asm target-specific error messages.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: lib/Sema/SemaStmtAsm.cpp:256-263
   // If we're compiling CUDA file and function attributes indicate that it's 
not
   // for this compilation side, skip all the checks.
   if (!DeclAttrsMatchCUDAMode(getLangOpts(), getCurFunctionDecl())) {
 GCCAsmStmt *NS = new (Context) GCCAsmStmt(
 Context, AsmLoc, IsSimple, IsVolatile, NumOutputs, NumInputs, Names,
 Constraints, Exprs.data(), AsmString, NumClobbers, Clobbers, 
RParenLoc);
 return NS;

ABataev wrote:
> tra wrote:
> > Now that inline asm errors are delayed, do we still need this check?
> With this patch, it is going to be delayed only for OpenMP. After the commit, 
> we could extend it for CUDA and remove this code. But not at the moment.
SGTM. 


Repository:
  rC Clang

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

https://reviews.llvm.org/D58243



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

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a reviewer: Charusso.
NoQ marked an inline comment as done.
NoQ added a subscriber: Charusso.
NoQ added a comment.

In D58367#1402796 , @Szelethus wrote:

> 2. I guess most of the visitors could be changed to this format, do you have 
> plans to convert them? I'll happily land a hand, because it sounds like a big 
> chore. I guess that would also test this implementation fairly well.


I don't have an immediate plan but i'll definitely convert visitors when i 
touch them and get annoyed. Also i believe that this new functionality is much 
more useful for //core// visitors than for checker visitors, simply because 
there's much more information to reverse-engineer in every visitor. Eg., 
`trackExpressionValue` would have been so much easier if it didn't have to 
figure out where did an assignment happen, but instead relied on `ExprEngine` 
to write down this sort of info as a tag in, say, `evalStore`. There are just 
too many ways to introduce a store/environment binding that represents moving a 
value from one place to another and it hurts me when i see all of them 
duplicated in the visitor as a military-grade portion of spaghetti. The same 
apples to the `ConditionBRVisitor` that might probably even benefit from having 
`ConstraintManager` explain the high-level assumption that's being made //as// 
it's being made; it might have also made @Charusso's work on supporting various 
sorts of assumptions much easier. At the same time, there aren't that many ways 
to close a file descriptor, so these are much easier to catch and explain in a 
visitor, so the main problem in checkers is the boilerplate. Checker APIs, 
however, are much more important to get polished because they're used much more 
often.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:222-236
+  /// Produce a program point tag that displays an additional path note
+  /// to the user. This is a lightweirght alternative to the
+  /// BugReporterVisitor mechanism: instead of visiting the bug report
+  /// node-by-node to restore the sequence of events that led to discovering
+  /// a bug, you can add notes as you add your transitions.
+  const NoteTag *getNoteTag(NoteTag::Callback &&Cb) {
+return Eng.getNoteTags().getNoteTag(std::move(Cb));

Note: you can't use this API in checker callbacks that don't provide a 
`CheckerContext`. Let's have a quick look at the list of such callbacks:
- `checkEndAnalysis()`. I'm not super worried about this one because it's a 
really weird place to put an //intermediate// note. If someone really really 
needs this, it should be possible to access `ExprEngine` directly from this 
callback.
- `evalAssume()`. This one's a bummer - it could really benefit from the new 
functionality, but we can't squeeze a checker context into it because it 
definitely doesn't make sense to add transitions in the middle of `assume()`. 
We should probably be able to allow returning a note tag from that callback 
somehow.
- `checkLiveSymbols()`. I'm not worried about this one because it doesn't 
correspond to any actual event in the program and there's no way to change the 
program state at this point. If we want to extract some information from it 
anyway, i guess we can add opaque checker-specific data into `SymbolReaper` and 
transfer it to `checkDeadSymbols()`.
- `checkPointerEscape()`, `checkRegionChanges()`. These are usually used for 
invalidation that normally doesn't deserve a note. But it can be argued that it 
may deserve a note sometimes (eg., "note: function call might have changed the 
value of a global variable"), so i guess i'm a tiny bit worried, but we can 
have a closer look at this when we find any actual examples.
- `checkEndOfTranslationUnit()`, `checkASTDecl()`,`checkASTCodeBody()`. These 
don't fire during path-sensitive analysis, so there's nothing to worry about.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58367



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

[PATCH] D58404: [clang-format] Add basic support for formatting C# files

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: djasper, klimek, krasimir, benhamilton, 
JonasToth.
MyDeveloperDay added a project: clang.
Herald added subscribers: jdoerfert, mgorny.

This revision adds basic support for formatting C# files with clang-format, I 
know the barrier to entry is high here  so I'm sending this revision in to test 
the water as to whether this might be something we'd consider landing.

Justification:
C# code just looks ugly in comparison to the C++ code in our source tree which 
is clang-formatted.

I've struggled with Visual Studio reformatting to get a clean and consistent 
style, I want to format our C# code on saving like I do now for C++ and i want 
it to have the same style as defined in our .clang-format file, so it 
consistent as it can be with C++.  (Braces/Breaking/Spaces/Indent etc..)

Using clang format without this patch leaves the code in a bad state, sometimes 
when the BreakStringLiterals is set, it  fails to compile.

Mostly the C# is similar to Java, except instead of JavaAnnotations I try to 
reuse the TT_AttributeSquare.

Almost the most valuable portion is to have a new Language in order to 
partition the configuration for C# within a common .clang-format file, with the 
auto detection on the .cs extension. But there are other C# specific styles 
that could be added later if this is accepted. in particular how  `{ set;get }` 
is formatted.

I'm including an example of its usage Before/After to demonstrate it initial 
capability.

C# code, formatted with current clang-format

  using System;
  using System.Collections.Generic;
  using System.Linq;
  using System.Text;
  using System.Threading.Tasks;
  using System.Reflection;
  
  namespace ConsoleApp1 {
  class Program {
  
[MainAttribute] static void Main(string[] args) {}
  
  public
void foo() {
  float f = 1.0;
  int a = (int)f;
  string[] args = new string[1];
  
  args[a] = "Hello";
}
  
  public
string Foo {
  set;
  get;
}
  }
  
  [ClassAttribute] public class Bar {
  public
Bar(){}
  
[MethodAttribute] public bool foo() {
  return true;
}
  
[XmlElement(ElementName = "TaxRate")] public int taxRate;
  
[MethodAttribute] internal bool foo() {
  string longstring =
  "VeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongLongLongLong";
  return true;
}
  }
  
  internal class InternalBar {
  public
InternalBar() {}
  }
  
  [TestClass][DeploymentItem("testData")] public class Test {
  }
  }

Same C# code, formatted with current clang-format with this revision  (no 
.clang-format file, out-of-box formatting)

  using System;
  using System.Collections.Generic;
  using System.Linq;
  using System.Text;
  using System.Threading.Tasks;
  using System.Reflection;
  
  namespace ConsoleApp1 {
  class Program {
  
[MainAttribute]
static void
Main(string[] args) {}
  
public void foo() {
  float f = 1.0;
  int a = (int) f;
  string[] args = new string[1];
  
  args[a] = "Hello";
}
  
public string Foo {
  set;
  get;
}
  }
  
  [ClassAttribute]
  public class Bar {
public Bar(){}
  
[MethodAttribute]
public bool foo() {
  return true;
}
  
[XmlElement(ElementName = "TaxRate")]
public int taxRate;
  
[MethodAttribute]
internal bool
foo() {
  string longstring =
  "VeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongLongLongLong";
  return true;
}
  }
  
  internal class InternalBar {
public InternalBar() {}
  }
  
  [TestClass]
  [DeploymentItem("testData")]
  public class Test {}
  }




Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58404

Files:
  docs/ClangFormat.rst
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  tools/clang-format/ClangFormat.cpp
  unittests/Format/CMakeLists.txt
  unittests/Format/FormatTestCSharp.cpp

Index: unittests/Format/FormatTestCSharp.cpp
===
--- /dev/null
+++ unittests/Format/FormatTestCSharp.cpp
@@ -0,0 +1,100 @@
+//===- unittest/Format/FormatTestCSharp.cpp - Formatting tests for CSharp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "FormatTestUtils.h"
+#include "clang/Format/Format.h"
+#include "llvm/Support/Debug.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+
+class FormatTestCSharp : public ::testing::Test {
+protected:
+  static s

[PATCH] D58243: [OPENMP] Delay emission of the asm target-specific error messages.

[Kelvin Li via Phabricator via cfe-commits]

kkwli0 added a comment.

The change looks okay to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58243



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

[PATCH] D54880: Ignore gcc's stack-clash-protection flag

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.
Herald added a project: clang.

See D42593 , I don't think it's good to ignore 
security flags like this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54880



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

[PATCH] D42593: GCC compatibility: Ignore -fstack-clash-protection

[Tom Stellard via Phabricator via cfe-commits]

tstellar abandoned this revision.
tstellar added a comment.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.

I agree with Joerg, I don't think we should be ignoring these kinds of security 
flags (even though we already ignore -fstack-check).


Repository:
  rC Clang

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

https://reviews.llvm.org/D42593



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

[PATCH] D58375: [Clang][NewPM] Disable tests that are broken under new PM

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I can understand why tests that use -O1 or -O2 would produce different results 
with the new pass manager, but it looks like not all the tests are like that.  
Do you know why those tests are failing?

For the tests that do use -O, instead of marking them unsupported, could you 
use -fno-experimental-new-pass-manager or something like that?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58375



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

[PATCH] D58065: [analyzer] Document the frontend library

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added inline comments.



Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:12
+This document will describe the frontend of the Static Analyzer, basically
+everything from compiling the analyzer from source, through it's invocation up
+to the beginning of the analysis. It will touch on topics such as

typo: it's --> its



Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:42
+
+The following section is sort of a summary, and severeral items will be later
+revisited in greater detail.

typo: severeral --> several


Repository:
  rC Clang

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

https://reviews.llvm.org/D58065



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

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang updated this revision to Diff 187466.

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

https://reviews.llvm.org/D50488

Files:
  docs/analyzer/checkers.rst
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
  test/Analysis/ptr-sort.cpp
  www/analyzer/alpha_checks.html

Index: www/analyzer/alpha_checks.html
===
--- www/analyzer/alpha_checks.html
+++ www/analyzer/alpha_checks.html
@@ -33,6 +33,7 @@
 OS X Alpha Checkers
 Security Alpha Checkers
 Unix Alpha Checkers
+Non-determinism Alpha Checkers
 
 
 
@@ -1174,6 +1175,28 @@
 
 
 
+
+Non-determinism Alpha Checkers
+
+
+Name, DescriptionExample
+
+
+
+alpha.nondeterminism.PointerSorting
+(C++)
+Check for non-determinism caused by sorting of pointers.
+
+
+// C++
+void test() {
+ int a = 1, b = 2;
+ std::vector V = {&a, &b};
+ std::sort(V.begin(), V.end()); // warn
+}
+
+
+
  
  
 
Index: test/Analysis/ptr-sort.cpp
===
--- /dev/null
+++ test/Analysis/ptr-sort.cpp
@@ -0,0 +1,57 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.nondeterminism.PointerSorting %s -analyzer-output=text -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+namespace std {
+  template
+  bool is_sorted(ForwardIt first, ForwardIt last);
+
+  template 
+  void nth_element(RandomIt first, RandomIt nth, RandomIt last);
+
+  template
+  void partial_sort(RandomIt first, RandomIt middle, RandomIt last);
+
+  template
+  void sort (RandomIt first, RandomIt last);
+
+  template
+  void stable_sort(RandomIt first, RandomIt last);
+
+  template
+  BidirIt partition(BidirIt first, BidirIt last, UnaryPredicate p);
+
+  template
+  BidirIt stable_partition(BidirIt first, BidirIt last, UnaryPredicate p);
+}
+
+bool f (int x) { return true; }
+bool g (int *x) { return true; }
+
+void PointerSorting() {
+  int a = 1, b = 2, c = 3;
+  std::vector V1 = {a, b};
+  std::vector V2 = {&a, &b};
+
+  std::is_sorted(V1.begin(), V1.end());// no-warning
+  std::nth_element(V1.begin(), V1.begin() + 1, V1.end());  // no-warning
+  std::partial_sort(V1.begin(), V1.begin() + 1, V1.end()); // no-warning
+  std::sort(V1.begin(), V1.end()); // no-warning
+  std::stable_sort(V1.begin(), V1.end());  // no-warning
+  std::partition(V1.begin(), V1.end(), f); // no-warning
+  std::stable_partition(V1.begin(), V1.end(), g);  // no-warning
+
+  std::is_sorted(V2.begin(), V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  std::nth_element(V2.begin(), V2.begin() + 1, V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  std::partial_sort(V2.begin(), V2.begin() + 1, V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  std::sort(V2.begin(), V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  std::stable_sort(V2.begin(), V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  std::partition(V2.begin(), V2.end(), f); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  std::stable_partition(V2.begin(), V2.end(), g); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+  // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+}
Index: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
===
--- /dev/null
+++ lib/StaticAnaly

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

> It's because it invokes CodeChecker, which by default enables 
> valist.Uninitialized, but not ValistBase. Do you have assert failures after 
> my hotfix?

@Szelethus Thanks. I run into this assert when run through CodeChecker:

  Assertion `CheckerTags.count(tag) != 0 && "Requested checker is not 
registered! Maybe you should add it as a " "dependency in Checkers.td?"'

Is there a workaround?


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

https://reviews.llvm.org/D50488



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

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

Also I don't see the helptext for PointerSorting when I run:

  clang -cc1 -analyzer-checker-help | grep Pointer
  
alpha.core.PointerArithmCheck for pointer arithmetic on locations 
other than array elements
alpha.core.PointerSub   Check for pointer subtractions on two 
pointers pointing to different memory chunks
alpha.nondeterminism.PointerSorting
cplusplus.InnerPointer  Check for inner pointers of C++ containers 
used after re/deallocation


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

https://reviews.llvm.org/D50488



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