[PATCH] D46300: [Clang] Implement function attribute no_stack_protector.

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta created this revision.
manojgupta added reviewers: aaron.ballman, rsmith, rnk, void.

This attribute tells clang to skip this function from stack protector
when -stack-protector option is passed.
GCC option for this is:
__attribute__((__optimize__("no-stack-protector"))) and the
equivalent clang syntax would be: __attribute__((no_stack_protector))

This is used in Linux kernel to selectively disable stack protector
in certain functions.


Repository:
  rC Clang

https://reviews.llvm.org/D46300

Files:
  include/clang/Basic/Attr.td
  lib/CodeGen/CodeGenModule.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGen/stack-protector.c


Index: test/CodeGen/stack-protector.c
===
--- test/CodeGen/stack-protector.c
+++ test/CodeGen/stack-protector.c
@@ -22,6 +22,14 @@
   printf("%s\n", a);
 }
 
+// DEF: define {{.*}}void @test2(i8* %msg) #[[B:.*]] {
+__attribute__((no_stack_protector))
+void test2(const char *msg) {
+  char a[strlen(msg) + 1];
+  strcpy(a, msg);
+  printf("%s\n", a);
+}
+
 // NOSSP-NOT: attributes #[[A]] = {{.*}} ssp
 // SSP: attributes #[[A]] = {{.*}} ssp{{ }}
 // SSPSTRONG: attributes #[[A]] = {{.*}} sspstrong
@@ -33,3 +41,15 @@
 // SAFESTACK-SSP: attributes #[[A]] = {{.*}} safestack ssp{{ }}
 // SAFESTACK-SSPSTRONG: attributes #[[A]] = {{.*}} safestack sspstrong
 // SAFESTACK-SSPREQ: attributes #[[A]] = {{.*}} safestack sspreq
+
+// NOSSP-NOT: attributes #[[B]] = {{.*}} ssp
+// SSP-NOT: attributes #[[B]] = {{.*}} ssp{{ }}
+// SSPSTRONG-NOT: attributes #[[B]] = {{.*}} sspstrong
+// SSPREQ-NOT: attributes #[[B]] = {{.*}} sspreq
+
+// SAFESTACK-SSP: attributes #[[B]] = {{.*}} safestack
+// SAFESTACK-SSP-NOT: attributes #[[B]] = {{.*}} safestack ssp{{ }}
+// SAFESTACK-SSPSTRONG: attributes #[[B]] = {{.*}} safestack
+// SAFESTACK-SSPSTRONG-NOT: attributes #[[B]] = {{.*}} safestack sspstrong
+// SAFESTACK-SSPREQ: attributes #[[B]] = {{.*}} safestack
+// SAFESTACK-SSPREQ-NOT: attributes #[[B]] = {{.*}} safestack sspreq
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6230,6 +6230,10 @@
   case AttributeList::AT_NoInstrumentFunction: // Interacts with -pg.
 handleSimpleAttribute(S, D, AL);
 break;
+  case AttributeList::AT_NoStackProtector:
+// Interacts with -fstack-protector options.
+handleSimpleAttribute(S, D, AL);
+break;
   case AttributeList::AT_StdCall:
   case AttributeList::AT_CDecl:
   case AttributeList::AT_FastCall:
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1142,12 +1142,14 @@
   if (!hasUnwindExceptions(LangOpts))
 B.addAttribute(llvm::Attribute::NoUnwind);
 
-  if (LangOpts.getStackProtector() == LangOptions::SSPOn)
-B.addAttribute(llvm::Attribute::StackProtect);
-  else if (LangOpts.getStackProtector() == LangOptions::SSPStrong)
-B.addAttribute(llvm::Attribute::StackProtectStrong);
-  else if (LangOpts.getStackProtector() == LangOptions::SSPReq)
-B.addAttribute(llvm::Attribute::StackProtectReq);
+  if (!D || !D->hasAttr()) {
+if (LangOpts.getStackProtector() == LangOptions::SSPOn)
+  B.addAttribute(llvm::Attribute::StackProtect);
+else if (LangOpts.getStackProtector() == LangOptions::SSPStrong)
+  B.addAttribute(llvm::Attribute::StackProtectStrong);
+else if (LangOpts.getStackProtector() == LangOptions::SSPReq)
+  B.addAttribute(llvm::Attribute::StackProtectReq);
+  }
 
   if (!D) {
 // If we don't have a declaration to control inlining, the function isn't
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1490,6 +1490,12 @@
   let Documentation = [NotTailCalledDocs];
 }
 
+def NoStackProtector : InheritableAttr {
+  let Spellings = [GCC<"no_stack_protector">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];
+}
+
 def NoThrow : InheritableAttr {
   let Spellings = [GCC<"nothrow">, Declspec<"nothrow">];
   let Subjects = SubjectList<[Function]>;


Index: test/CodeGen/stack-protector.c
===
--- test/CodeGen/stack-protector.c
+++ test/CodeGen/stack-protector.c
@@ -22,6 +22,14 @@
   printf("%s\n", a);
 }
 
+// DEF: define {{.*}}void @test2(i8* %msg) #[[B:.*]] {
+__attribute__((no_stack_protector))
+void test2(const char *msg) {
+  char a[strlen(msg) + 1];
+  strcpy(a, msg);
+  printf("%s\n", a);
+}
+
 // NOSSP-NOT: attributes #[[A]] = {{.*}} ssp
 // SSP: attributes #[[A]] = {{.*}} ssp{{ }}
 // SSPSTRONG: attributes #[[A]] = {{.*}} sspstrong
@@ -33,3 +41,15 @@
 // SAFESTACK-SSP: attributes #[[A]] = {{.*}} safestack ssp{{ }}
 // SAFESTACK-SSPSTRONG: attributes #[[A]] = {{.*}} safestack sspstrong
 // 

r331244 - Implement P0482R2, support for char8_t type.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Apr 30 22:02:45 2018
New Revision: 331244

URL: http://llvm.org/viewvc/llvm-project?rev=331244=rev
Log:
Implement P0482R2, support for char8_t type.

This is not yet part of any C++ working draft, and so is controlled by the flag
-fchar8_t rather than a -std= flag. (The GCC implementation is controlled by a
flag with the same name.)

This implementation is experimental, and will be removed or revised
substantially to match the proposal as it makes its way through the C++
committee.

Added:
cfe/trunk/test/CodeGenCXX/char8_t.cpp
cfe/trunk/test/Lexer/char8_t.cpp
cfe/trunk/test/SemaCXX/char8_t.cpp
Modified:
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/AST/BuiltinTypes.def
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Basic/Specifiers.h
cfe/trunk/include/clang/Basic/TokenKinds.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Sema/DeclSpec.h
cfe/trunk/include/clang/Sema/Initialization.h
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/lib/AST/MicrosoftMangle.cpp
cfe/trunk/lib/AST/NSAPI.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/AST/TypeLoc.cpp
cfe/trunk/lib/Analysis/PrintfFormatString.cpp
cfe/trunk/lib/Basic/IdentifierTable.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Format/FormatToken.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/lib/Index/USRGeneration.cpp
cfe/trunk/lib/Lex/PPExpressions.cpp
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseExpr.cpp
cfe/trunk/lib/Parse/ParseExprCXX.cpp
cfe/trunk/lib/Parse/ParseTentative.cpp
cfe/trunk/lib/Sema/DeclSpec.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/lib/Serialization/ASTCommon.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/test/Lexer/cxx-features.cpp

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=331244=331243=331244=diff
==
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Mon Apr 30 22:02:45 2018
@@ -999,6 +999,7 @@ public:
   CanQualType WCharTy;  // [C++ 3.9.1p5].
   CanQualType WideCharTy; // Same as WCharTy in C++, integer type in C99.
   CanQualType WIntTy;   // [C99 7.24.1], integer type unchanged by default 
promotions.
+  CanQualType Char8Ty;  // [C++20 proposal]
   CanQualType Char16Ty; // [C++0x 3.9.1p5], integer type in C99.
   CanQualType Char32Ty; // [C++0x 3.9.1p5], integer type in C99.
   CanQualType SignedCharTy, ShortTy, IntTy, LongTy, LongLongTy, Int128Ty;

Modified: cfe/trunk/include/clang/AST/BuiltinTypes.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/BuiltinTypes.def?rev=331244=331243=331244=diff
==
--- cfe/trunk/include/clang/AST/BuiltinTypes.def (original)
+++ cfe/trunk/include/clang/AST/BuiltinTypes.def Mon Apr 30 22:02:45 2018
@@ -72,6 +72,9 @@ UNSIGNED_TYPE(UChar, UnsignedCharTy)
 // 'wchar_t' for targets where it's unsigned
 SHARED_SINGLETON_TYPE(UNSIGNED_TYPE(WChar_U, WCharTy))
 
+// 'char8_t' in C++20 (proposed)
+UNSIGNED_TYPE(Char8, Char8Ty)
+
 // 'char16_t' in C++
 UNSIGNED_TYPE(Char16, Char16Ty)
 

Modified: cfe/trunk/include/clang/AST/Type.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=331244=331243=331244=diff
==
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Mon Apr 30 22:02:45 2018
@@ -1777,6 +1777,7 @@ public:
   bool isBooleanType() const;
   bool isCharType() const;
   bool isWideCharType() const;
+  bool isChar8Type() const;
   bool isChar16Type() const;
   bool isChar32Type() const;
   bool isAnyCharacterType() const;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=331244=331243=331244=diff
==
--- 

[libcxx] r331241 - Fix return type of isinf(double) and isnan(double) where possible.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Apr 30 20:05:40 2018
New Revision: 331241

URL: http://llvm.org/viewvc/llvm-project?rev=331241=rev
Log:
Fix return type of isinf(double) and isnan(double) where possible.

When using an old version of glibc, a ::isinf(double) and ::isnan(double)
function is provided, rather than just the macro required by C and C++.
Displace this function using _LIBCPP_PREFERRED_OVERLOAD where possible.

The only remaining case where we should get the wrong return type is now
glibc + libc++ + a non-clang compiler.

Modified:
libcxx/trunk/include/math.h
libcxx/trunk/test/std/numerics/c.math/cmath.pass.cpp

Modified: libcxx/trunk/include/math.h
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/math.h?rev=331241=331240=331241=diff
==
--- libcxx/trunk/include/math.h (original)
+++ libcxx/trunk/include/math.h Mon Apr 30 20:05:40 2018
@@ -483,6 +483,20 @@ typename std::enable_if<
 isinf(_A1) _NOEXCEPT
 { return false; }
 
+#ifdef _LIBCPP_PREFERRED_OVERLOAD
+inline _LIBCPP_INLINE_VISIBILITY
+bool
+isinf(float __lcpp_x) _NOEXCEPT { return __libcpp_isinf(__lcpp_x); }
+
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_PREFERRED_OVERLOAD
+bool
+isinf(double __lcpp_x) _NOEXCEPT { return __libcpp_isinf(__lcpp_x); }
+
+inline _LIBCPP_INLINE_VISIBILITY
+bool
+isinf(long double __lcpp_x) _NOEXCEPT { return __libcpp_isinf(__lcpp_x); }
+#endif
+
 #endif  // isinf
 
 // isnan
@@ -513,6 +527,20 @@ typename std::enable_ifhttp://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/numerics/c.math/cmath.pass.cpp?rev=331241=331240=331241=diff
==
--- libcxx/trunk/test/std/numerics/c.math/cmath.pass.cpp (original)
+++ libcxx/trunk/test/std/numerics/c.math/cmath.pass.cpp Mon Apr 30 20:05:40 
2018
@@ -661,11 +661,12 @@ void test_isinf()
 static_assert((std::is_same::value), 
"");
 
 typedef decltype(std::isinf((double)0)) DoubleRetType;
-#ifndef __linux__
+#if !defined(__linux__) || defined(__clang__)
 static_assert((std::is_same::value), "");
 #else
-// GLIBC < 2.26 defines 'isinf(double)' with a return type of 'int' in
-// all C++ dialects. The test should tolerate this.
+// GLIBC < 2.23 defines 'isinf(double)' with a return type of 'int' in
+// all C++ dialects. The test should tolerate this when libc++ can't work
+// around it.
 // See: https://sourceware.org/bugzilla/show_bug.cgi?id=19439
 static_assert((std::is_same::value
 || std::is_same::value), "");
@@ -746,11 +747,12 @@ void test_isnan()
 static_assert((std::is_same::value), 
"");
 
 typedef decltype(std::isnan((double)0)) DoubleRetType;
-#ifndef __linux__
+#if !defined(__linux__) || defined(__clang__)
 static_assert((std::is_same::value), "");
 #else
-// GLIBC < 2.26 defines 'isnan(double)' with a return type of 'int' in
-// all C++ dialects. The test should tolerate this.
+// GLIBC < 2.23 defines 'isinf(double)' with a return type of 'int' in
+// all C++ dialects. The test should tolerate this when libc++ can't work
+// around it.
 // See: https://sourceware.org/bugzilla/show_bug.cgi?id=19439
 static_assert((std::is_same::value
 || std::is_same::value), "");


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

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Had a look at the actual code, came up with some comments, most are fairly 
minor, so good job! You did a good job explaining the overall idea, but a lot 
of specifics were difficult to understand, so i made a few comments on how to 
make the code a bit easier to read.

> I know, and I tried to look for ways to split this checker into smaller 
> logical chunks, but I didn't find a satisfying solution.

What i would have done:

1. A checker that straightforwardly iterates through immediate fields of the 
current object and reports uninitialized fields.
2. Add base classes support.
3. Heuristic for skipping sub-constructors could use a separate review.
4. Start skipping arrays and unions.
5. Add simple pointer dereference support - introduce chains of fields.
6. More heuristics - the one for symbolic regions, the one for system header 
fields, etc.

etc.

I'm asking for this because it's, like, actually difficult and painful to 
understand formal information in large chunks. One-solution-at-a-time would 
have been so much easier. It is really a bad idea to start by presenting a 
working solution; the great idea is to present a small non-working solution 
with a reasonable idea behind it, and then extend it "in place" as much as it 
seems necessary. It is very comfortable when parts of the patch with different 
"status" (main ideas, corner-case fixes, heuristics, refactoring) stay in 
separate patches. Alpha checkers are essentially branches: because our svn 
doesn't have branches, we have alpha checkers and off-by-default flags. Even 
before step 1, you should be totally fine with committing an empty checker with 
a single empty callback and a header comment - even on such "step 0." we would 
be able to discuss if your approach to the checker seems right or might have 
inevitable issues, and it could have saved a lot of time.

Essentially every part of the solution that you implement, if you think it 
might deserve a separate discussion, also deserves a separate patch. It is 
reasonable to split the work into logical chunks before you start it. It's very 
unlikely that you have a single idea that takes 500 non-trivial lines of code 
to express. Splitting up ideas so that they were easy to grasp is an invaluable 
effort. I had very positive experience with that recently, both as a coder and 
as a reviewer, so i encourage that a lot.




Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:47
+class FieldChainInfo {
+  using FieldChain = llvm::SmallVector;
+

At a glance it looks like a great use case for an immutable list. It's easy to 
use the immutable list as a stack, and it's also O(1) to copy/move and take 
snapshots of it.

Note that you copy 10 pointers every time you copy //or move// a `SmallVector`. 
Small vectors don't make much sense as fields of actively used data structures 
- their main purpose is to live on the stack as local variables.



Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:51-53
+  /// If this is a fieldchain whose last element is an uninitialized region of 
a
+  /// pointer type, this variable will store whether the pointer itself or the
+  /// pointee is uninitialized.

Please move this doc to the getter/setter function. That's where people expect 
to find it when they're reading code.



Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:58-59
+  FieldChainInfo() = default;
+  /// Delegates to the copy ctor and adds FR to Chain. Note that Chain cannot 
be
+  /// modified in a FieldChainInfo object after its construction.
+  FieldChainInfo(const FieldChainInfo , const FieldRegion *FR);

Do we really need the other field (`IsDereferenced`) to be mutable? Or maybe we 
can keep the whole object immutable? I.e., instead of `dereference()` we'll get 
a constructor that constructs a `FieldChainInfo` with the same chain but with 
the dereferenced flag set.



Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:86-89
+  StoreManager 
+  MemRegionManager 
+  SourceManager 
+  Store S;

All of these can be replaced with a single `ProgramStateRef` object.



Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:93
+  const bool IsPedantic;
+  bool IsChecked = false;
+  bool IsAnyFieldInitialized = false;

This field is worth documenting.



Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:156-157
+  //
+  // We'll traverse each node of the above graph with the appropiate one of
+  // these methods:
+  bool isUnionUninit(const TypedValueRegion *R);

Please explain what these methods do in a more direct manner. Something like 
"This method returns true if an uninitialized field was found within the 
region. It should only be used with regions of union-type 

[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

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

rsmith added inline comments.



Comment at: include/clang/Lex/PPCallbacks.h:263
+  /// \brief Callback invoked when a has_include directive is read.
+  virtual void HasInclude(SourceLocation Loc, const FileEntry *File) {
+  }

This callback seems pretty unhelpful in the case where lookup of the file 
failed (you just get a source location). Could we pass in the `FileName` and 
`IsAngled` flag too (like we do for `InclusionDirective`)? I think that's what 
(eg) a callback tracking anti-dependencies (for a more sophisticated build 
system that can handle them) would want.



Comment at: lib/Frontend/DependencyFile.cpp:325
+void DFGImpl::HasInclude(SourceLocation Loc, const FileEntry *File) {
+  if (!File)
+return;

Have you thought about whether we should add a dependency even for a missing 
file under `-MG` (`AddMissingHeaderDeps`) mode? I think it's probably better to 
not do so (ie, the behavior in this patch), but it seems worth considering.


https://reviews.llvm.org/D30882



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

[PATCH] D30881: Track skipped files in dependency scanning

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

I plan to look into cleaning this up and addressing the review comments.

Need to check if `DepCollectorPPCallbacks` has to be updated too.




Comment at: lib/Frontend/DependencyFile.cpp:302
+ SrcMgr::CharacteristicKind FileType) {
+  AddFilename(llvm::sys::path::remove_leading_dotslash(SkippedFile.getName()));
+}

I suspect we need to call `FileMatchesDepCriteria` here too.


https://reviews.llvm.org/D30881



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

[PATCH] D46287: [Driver] Don't add -dwarf-column-info when using -gcodeview on non-msvc targets

[Shoaib Meenai via Phabricator via cfe-commits]

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

Makes sense to me. You may wanna wait for @zturner, but I can't really imagine 
any benefit to restricting this to MSVC.


Repository:
  rC Clang

https://reviews.llvm.org/D46287



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

[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

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

Quuxplusone added a comment.

In https://reviews.llvm.org/D43322#1083075, @arthur.j.odwyer wrote:

> Sorry, I responded via email but I guess Phabricator didn't pick it up for
>  some reason.
>  See below.


And then Phab *still* didn't pick up the important part... Okay, I'll try 
pasting it here.




Can you post the diagnostic exactly as it appears in the compiler output? I am 
surprised that it would appear here. It should appear here only if the standard 
library's implicit conversion from std::map<...>::iterator to 
std::map<...>::const_iterator were implemented as a conversion operator instead 
of as a converting constructor. I have verified that both libstdc++ trunk and 
libc++ trunk implement it "correctly" as a converting constructor, and I have 
verified on Wandbox/Godbolt that no warning is emitted on your sample code when 
using either of those standard libraries.

Is it possible that you are using a standard library that is neither libc++ or 
libstdc++?
Is it possible that that standard library implements the 
iterator-to-const_iterator conversion as a conversion operator instead of a 
converting constructor?

> And then someone else pointed out that 
> http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1579 might mean 
> that the code-as is should be fine too in C++14 – I don't know if that's 
> true, but maybe you could comment on that too :-)

CWG1579 is the defect report against ISO C++11 which is mentioned in the 
wording of the off-by-default -Wreturn-std-move-in-c++11. :)  Anything that 
-Wreturn-std-move itself complains about, is stuff that was NOT fixed by 
CWG1579.
Alternative hypothesis: Is it possible that you have manually turned on 
-Wreturn-std-move-in-c++11 (which is deliberately omitted from -Wmove/-Wall)? 
In that case, the warning is absolutely correct, but you are probably wrong to 
care about fixing it in this case. :)

> Maybe the warning could suggest something like "cv qualifiers don't match, 
> make them match" on a note in addition or instead of std::move() for this 
> case?

I would love to see this, but I'm not comfortable trying to add it myself. 
Remember, the problem here isn't core-language cv-qualifiers ("iterator" vs. 
"const iterator"); the problem is library-level type mismatch ("iterator" vs. 
"const_iterator"). You'd have to somehow look at the human-readable name of the 
typedef and determine that it was a match for the regex /const_(.*)/ and the 
target type was a match for /\1/, and then you'd have to generate a meaningful 
note.


Repository:
  rC Clang

https://reviews.llvm.org/D43322



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

[PATCH] D45177: CStringChecker, check strlcpy/strlcat

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Whoops - this isn't quite correct because there's one more difference between 
strlcpy/strlcat and the standard strcpy/strcat/strncpy/strncat: the return 
value. After this patch the new functions are modeled as if they return a 
pointer into the string, which is incorrect and in fact causes crashes.

One of the crashes is on the following code:

  int foo(char *d) {
char e[1];
return strlcpy(e, d, sizeof(e)) >= sizeof(e);
  }

...when analyzed as `clang -cc1 -w -analyze -analyzer-checker=core,unix 
repro.c`.

David, would you be willing to have a look at this problem?

Also I forgot to add the tests before committing. Sorry!


Repository:
  rC Clang

https://reviews.llvm.org/D45177



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

r331238 - [ShadowCallStack] fix the docs

[Kostya Serebryany via cfe-commits]

Author: kcc
Date: Mon Apr 30 17:15:56 2018
New Revision: 331238

URL: http://llvm.org/viewvc/llvm-project?rev=331238=rev
Log:
[ShadowCallStack] fix the docs

Modified:
cfe/trunk/docs/ShadowCallStack.rst

Modified: cfe/trunk/docs/ShadowCallStack.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ShadowCallStack.rst?rev=331238=331237=331238=diff
==
--- cfe/trunk/docs/ShadowCallStack.rst (original)
+++ cfe/trunk/docs/ShadowCallStack.rst Mon Apr 30 17:15:56 2018
@@ -137,7 +137,7 @@ Generates the following x86_64 assembly
 .. code-block:: gas
 
 push   %rax
-callq  foo
+callq  bar
 add$0x1,%eax
 pop%rcx
 retq
@@ -165,7 +165,7 @@ assembly:
 mov%gs:(%r11),%r11
 mov%r10,%gs:(%r11)
 push   %rax
-callq  foo
+callq  bar
 add$0x1,%eax
 pop%rcx
 xor%r11,%r11


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

[PATCH] D45291: [Driver] Infer Android sysroot location.

[Stephen Hines via Phabricator via cfe-commits]

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

Thanks for adding this simplified support. Sorry about the extreme delay in 
getting these reviewed.


Repository:
  rC Clang

https://reviews.llvm.org/D45291



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

[PATCH] D46293: [clang-tidy/google-runtime-int] Allow passing non-bitwidth types to printf()-style APIs

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/google/IntegerTypesCheck.cpp:66
+  Finder->addMatcher(typeLoc(loc(isInteger()),
+ unless(hasAncestor(callExpr(
+ 
callee(functionDecl(hasAttr(attr::Format)))

I vaguely remember that there were some problems matching ancestors of 
TypeLocs. I hope this has been solved since then, but making a test more robust 
would be useful. See the other comment.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46293



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

[PATCH] D46293: [clang-tidy/google-runtime-int] Allow passing non-bitwidth types to printf()-style APIs

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG with one comment. Thank you for the fix!




Comment at: test/clang-tidy/google-runtime-int.cpp:77
+__attribute__((__format__ (__printf__, 1, 2)))
+void myprintf(const char* s, ...);
+

Please add another function similar to this one, but without the attribute and 
verify that the check fires on arguments to that function.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46293



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

r331233 - [Modules] Fix testcases from r331232

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Mon Apr 30 15:57:02 2018
New Revision: 331233

URL: http://llvm.org/viewvc/llvm-project?rev=331233=rev
Log:
[Modules] Fix testcases from r331232

Modified:

cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/module.modulemap
cfe/trunk/test/Modules/non-ambiguous-enum.m

Modified: 
cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/module.modulemap
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/module.modulemap?rev=331233=331232=331233=diff
==
--- 
cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/module.modulemap
 (original)
+++ 
cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/module.modulemap
 Mon Apr 30 15:57:02 2018
@@ -1,6 +1,5 @@
 
 framework module A {
   header "a.h"
-  //module * { export * }
   export *
 }

Modified: cfe/trunk/test/Modules/non-ambiguous-enum.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/non-ambiguous-enum.m?rev=331233=331232=331233=diff
==
--- cfe/trunk/test/Modules/non-ambiguous-enum.m (original)
+++ cfe/trunk/test/Modules/non-ambiguous-enum.m Mon Apr 30 15:57:02 2018
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t 
-F%S/Inputs/non-ambiguous-enum -fsyntax-only %s -verify
-#import 
-#import 
+#import 
+#import 
 
 // expected-no-diagnostics
 


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

r331225 - [docs] Fix docs/InternalsManual.rst heading.

[Fangrui Song via cfe-commits]

Author: maskray
Date: Mon Apr 30 13:51:50 2018
New Revision: 331225

URL: http://llvm.org/viewvc/llvm-project?rev=331225=rev
Log:
[docs] Fix docs/InternalsManual.rst heading.

Modified:
cfe/trunk/docs/InternalsManual.rst

Modified: cfe/trunk/docs/InternalsManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/InternalsManual.rst?rev=331225=331224=331225=diff
==
--- cfe/trunk/docs/InternalsManual.rst (original)
+++ cfe/trunk/docs/InternalsManual.rst Mon Apr 30 13:51:50 2018
@@ -423,7 +423,7 @@ Fix-it hints can be created with one of
 .. _DiagnosticConsumer:
 
 The ``DiagnosticConsumer`` Interface
-^^
+
 
 Once code generates a diagnostic with all of the arguments and the rest of the
 relevant information, Clang needs to know what to do with it.  As previously


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

[PATCH] D46293: [clang-tidy/google-runtime-int] Allow passing non-bitwidth types to printf()-style APIs

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: alexfh, bkramer.
Herald added subscribers: cfe-commits, klimek.

The `google-runtime-int` check currently fires on calls like:

  printf("%lu", (unsigned long)foo);

However, the style guide says:

> Where possible, avoid passing arguments of types specified by
>  bitwidth typedefs to printf-based APIs.

http://google.github.io/styleguide/cppguide.html#64-bit_Portability

This diff relaxes the check to not fire on parameters to functions
with the `__format__` attribute. (I didn't specifically check
for `__printf__` since there are a few variations.)

Test Plan: New tests added. Ran tests with:

  % make -j16 check-clang-tools


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46293

Files:
  clang-tidy/google/IntegerTypesCheck.cpp
  test/clang-tidy/google-runtime-int.cpp


Index: test/clang-tidy/google-runtime-int.cpp
===
--- test/clang-tidy/google-runtime-int.cpp
+++ test/clang-tidy/google-runtime-int.cpp
@@ -72,3 +72,11 @@
   B a, b;
   a = b;
 }
+
+__attribute__((__format__ (__printf__, 1, 2)))
+void myprintf(const char* s, ...);
+
+void doprint() {
+  uint64 foo = 23;
+  myprintf("foo %lu %lu", (unsigned long)42, (unsigned long)foo);
+}
Index: clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tidy/google/IntegerTypesCheck.cpp
@@ -11,6 +11,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/AttrKinds.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/TargetInfo.h"
@@ -56,7 +57,16 @@
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
   if (!getLangOpts().CPlusPlus)
 return;
-  Finder->addMatcher(typeLoc(loc(isInteger())).bind("tl"), this);
+  // Match any integer types, unless they are passed to a printf-based API:
+  //
+  // http://google.github.io/styleguide/cppguide.html#64-bit_Portability
+  // "Where possible, avoid passing arguments of types specified by
+  // bitwidth typedefs to printf-based APIs."
+  Finder->addMatcher(typeLoc(loc(isInteger()),
+ unless(hasAncestor(callExpr(
+ 
callee(functionDecl(hasAttr(attr::Format)))
+ .bind("tl"),
+ this);
   IdentTable = llvm::make_unique(getLangOpts());
 }
 


Index: test/clang-tidy/google-runtime-int.cpp
===
--- test/clang-tidy/google-runtime-int.cpp
+++ test/clang-tidy/google-runtime-int.cpp
@@ -72,3 +72,11 @@
   B a, b;
   a = b;
 }
+
+__attribute__((__format__ (__printf__, 1, 2)))
+void myprintf(const char* s, ...);
+
+void doprint() {
+  uint64 foo = 23;
+  myprintf("foo %lu %lu", (unsigned long)42, (unsigned long)foo);
+}
Index: clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tidy/google/IntegerTypesCheck.cpp
@@ -11,6 +11,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/AttrKinds.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/TargetInfo.h"
@@ -56,7 +57,16 @@
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
   if (!getLangOpts().CPlusPlus)
 return;
-  Finder->addMatcher(typeLoc(loc(isInteger())).bind("tl"), this);
+  // Match any integer types, unless they are passed to a printf-based API:
+  //
+  // http://google.github.io/styleguide/cppguide.html#64-bit_Portability
+  // "Where possible, avoid passing arguments of types specified by
+  // bitwidth typedefs to printf-based APIs."
+  Finder->addMatcher(typeLoc(loc(isInteger()),
+ unless(hasAncestor(callExpr(
+ callee(functionDecl(hasAttr(attr::Format)))
+ .bind("tl"),
+ this);
   IdentTable = llvm::make_unique(getLangOpts());
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGExprConstant.cpp:1395
 
+static bool isZeroInitializer(ConstantEmitter , const Expr *Init) {
+  QualType InitTy = Init->getType().getCanonicalType();

You should have a comment here clarifying that this is checking whether the 
initializer is equivalent to a C++ zero initialization, not checking whether 
the initializer produces a zero bit-pattern.



Comment at: lib/CodeGen/CGExprConstant.cpp:1414
+  return true;
+}
+  }

You should check the array fill expression here; for now, the test case would 
be something like `StructWithNonTrivialDefaultInit sArr[10] = {};`.



Comment at: lib/CodeGen/CGExprConstant.cpp:1415
+}
+  }
+  return false;

I would suggest doing your primary switch over the form of Init instead of its 
type, and you can just have an isIntegerConstantExpr check at the end.

You should also check for a null pointer expression.


Repository:
  rC Clang

https://reviews.llvm.org/D46241



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

[PATCH] D34440: [Clang] Expand response files before loading compilation database

[Chih-Hung Hsieh via Phabricator via cfe-commits]

chh added a comment.
Herald added a subscriber: llvm-commits.

FYI, Android NDK has another use case in 
https://github.com/android-ndk/ndk/issues/680.
It would be nice to have clang-tidy recognize the response file.


Repository:
  rL LLVM

https://reviews.llvm.org/D34440



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

[PATCH] D46190: For an ODR declaration, set the 'Used' bit on its associated declarations.

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

rsmith added a comment.

In https://reviews.llvm.org/D46190#1082616, @CarlosAlbertoEnciso wrote:

> My initial approach was based on the following test case:
>
>   void Bar() {
> typedef int I_Ref;
> I_Ref var_bar;
>   }
>  
>   void Foo() {
> typedef int I_Used;
> I_Used var_foo;
> var_foo = 2;
>   }
>  
>   void Test() {
> Foo();
>   }
>


[...]

> The typedef 'I_Ref' is marked as 'referenced' due to its association with 
> 'var_bar'.
>  The typedef 'I_Used' is marked as 'referenced' despite that its association 
> with 'var_foo' which is 'used'
>  Both 'typedefs' are marked as 'referenced'.

This is already the correct outcome. Both typedefs are referenced; this is 
correct because they are named by the source code. Neither typedef is marked as 
"used"; this is correct because a typedef cannot be odr-used.

Let's look at your original example again (I've cleaned it up and extended it a 
bit, but hopefully this matches your intent):

  namespace nsp {
int var1, var2;
  }
  
  using nsp::var1;
  using nsp::var2;
  
  void bar() {
nsp::var1 = 1;
var2 = 1;
  }

Now, what should happen here is:

- The use of `nsp::var1` in `bar` marks `nsp::var1` as referenced (because it 
was named), and marks `nsp::var1` as used (because it was odr-used by the 
assignment). The `UsingShadowDecl` for `::var1` is not marked as referenced 
(nor used), so we emit an unused-using-declaration warning.
- The use of `var2` in `bar` marks `::var2` as referenced (because it was 
named), and marks `nsp::var2` as used (because it was odr-used by the 
assignment). The `UsingShadowDecl` for `::var2` is marked as referenced, so we 
do not warn on it.


Repository:
  rC Clang

https://reviews.llvm.org/D46190



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

r331232 - [Modules] Handle ObjC/C ODR-like semantics for EnumConstantDecl

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Mon Apr 30 15:14:29 2018
New Revision: 331232

URL: http://llvm.org/viewvc/llvm-project?rev=331232=rev
Log:
[Modules] Handle ObjC/C ODR-like semantics for EnumConstantDecl

Support for ObjC/C ODR-like semantics with structural equivalence
checking was added back in r306918. There enums are handled and also
checked for structural equivalence. However, at use time of
EnumConstantDecl, support was missing for preventing ambiguous
name lookup.

Add the missing bits for properly merging EnumConstantDecl.

rdar://problem/38374569

Added:
cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/
cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/
cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/
cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a.h
cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a0.h
cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/

cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/module.modulemap
cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/B.framework/
cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/B.framework/Headers/
cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/B.framework/Headers/b.h
cfe/trunk/test/Modules/non-ambiguous-enum.m
Modified:
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=331232=331231=331232=diff
==
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Mon Apr 30 15:14:29 2018
@@ -2547,6 +2547,20 @@ void ASTDeclReader::mergeRedeclarable(Re
   }
 }
 
+/// ODR-like semantics for C/ObjC allow us to merge tag types and a structural
+/// check in Sema guarantees the types can be merged (see C11 6.2.7/1 or C89
+/// 6.1.2.6/1). Although most merging is done in Sema, we need to guarantee
+/// that some types are mergeable during deserialization, otherwise name
+/// lookup fails. This is the case for EnumConstantDecl.
+bool allowODRLikeMergeInC(NamedDecl *ND) {
+  if (!ND)
+return false;
+  // TODO: implement merge for other necessary decls.
+  if (dyn_cast(ND))
+return true;
+  return false;
+}
+
 /// \brief Attempts to merge the given declaration (D) with another declaration
 /// of the same entity, for the case where the entity is not actually
 /// redeclarable. This happens, for instance, when merging the fields of
@@ -2557,10 +2571,12 @@ void ASTDeclReader::mergeMergeable(Merge
   if (!Reader.getContext().getLangOpts().Modules)
 return;
 
-  // ODR-based merging is only performed in C++. In C, identically-named things
-  // in different translation units are not redeclarations (but may still have
-  // compatible types).
-  if (!Reader.getContext().getLangOpts().CPlusPlus)
+  // ODR-based merging is performed in C++ and in some cases (tag types) in C.
+  // Note that C identically-named things in different translation units are
+  // not redeclarations, but may still have compatible types, where ODR-like
+  // semantics may apply.
+  if (!Reader.getContext().getLangOpts().CPlusPlus &&
+  !allowODRLikeMergeInC(dyn_cast(static_cast(D
 return;
 
   if (FindExistingResult ExistingRes = findExisting(static_cast(D)))

Added: cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a.h?rev=331232=auto
==
--- cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a.h 
(added)
+++ cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a.h 
Mon Apr 30 15:14:29 2018
@@ -0,0 +1 @@
+#include 

Added: cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a0.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a0.h?rev=331232=auto
==
--- cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a0.h 
(added)
+++ cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Headers/a0.h 
Mon Apr 30 15:14:29 2018
@@ -0,0 +1 @@
+#include 

Added: 
cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/module.modulemap
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/module.modulemap?rev=331232=auto
==
--- 
cfe/trunk/test/Modules/Inputs/non-ambiguous-enum/A.framework/Modules/module.modulemap
 (added)
+++ 

r331231 - [CodeGen] Fix typo in comment form->from. NFC

[Craig Topper via cfe-commits]

Author: ctopper
Date: Mon Apr 30 15:02:48 2018
New Revision: 331231

URL: http://llvm.org/viewvc/llvm-project?rev=331231=rev
Log:
[CodeGen] Fix typo in comment form->from. NFC

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

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=331231=331230=331231=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Mon Apr 30 15:02:48 2018
@@ -1809,7 +1809,7 @@ void CodeGenModule::ConstructAttributeLi
 FuncAttrs.addAttribute(llvm::Attribute::NoReturn);
 
   // If we have information about the function prototype, we can learn
-  // attributes form there.
+  // attributes from there.
   AddAttributesFromFunctionProtoType(getContext(), FuncAttrs,
  CalleeInfo.getCalleeFunctionProtoType());
 


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

[PATCH] D46052: GNUstep Objective-C ABI version 2

[Dustin L. Howett via Phabricator via cfe-commits]

DHowett-MSFT added inline comments.



Comment at: lib/CodeGen/CGObjCGNU.cpp:439
+ ArrayRef IvarOffsets,
+ ArrayRef IvarAlign,
+ ArrayRef IvarOwnership);

While we're here, is there value in storing the ivar size in layout as well, so 
that the runtime doesn't need to calculate it from the distance to the next 
ivar/end of the instance?



Comment at: lib/CodeGen/CGObjCGNU.cpp:1402
+Stop->setVisibility(llvm::GlobalValue::HiddenVisibility);
+return { Start, Stop };
+  }

This should be readily expandable for PE/COFF, but we'll need to change some of 
the section names to fit better. Is it worth abstracting the names of the 
sections across the target format somehow?

(pe/coff will need to emit COMDAT symbols pointing into lexicographically 
sortable section names that the linker can fold away)



Comment at: lib/CodeGen/CGObjCGNU.cpp:1446
+/*isConstant*/true, llvm::GlobalValue::LinkOnceAnyLinkage,
+LoadFunction, ".objc_ctor");
+// Check that this hasn't been renamed.  This shouldn't happen, because

Is there a way to ensure that objc_load happens before other static 
initializers across the entire set of linker inputs?


Repository:
  rC Clang

https://reviews.llvm.org/D46052



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

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

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang added a comment.

> Why do you think that looping is required? From my understanding, we
>  need the first usage (DeclRefExpr) to get the analysis started. The
>  analysis itself will check all remaining uses. Is this necessary,
>  because we analysis on a `Expr` basis?

Yes. the analyzer traces starting from the given DeclRefExpr, e.g.:

  int a; // <- varDecl
  const int& b = a;  // <- first DeclRefExpr
  int& c = a; // <- second DeclRefExpr
  
  const int& b2 = b;
  int& c2 = c;
  c2 = 10;

If we start from the first DeclRefExpr, we'll only trace to `b` and then to 
`b2`. We need to start from varDecl (or loop over all DeclRefExpr) to be able 
to trace to the second DeclRefExpr from where we can get to `c` -> `c2` -> `c2 
= 10`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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

[PATCH] D46165: [Modules] Handle ObjC/C ODR-like semantics for EnumConstantDecl

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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



Comment at: lib/Serialization/ASTReaderDecl.cpp:2568-2570
   // ODR-based merging is only performed in C++. In C, identically-named things
   // in different translation units are not redeclarations (but may still have
+  // compatible types). However, clang is able to merge tag types with ODR-like

rsmith wrote:
> I think this comment would benefit from more rewriting, since we've basically 
> decided that we do want to apply the ODR merging rule to C.
Sure, will update that!


Repository:
  rC Clang

https://reviews.llvm.org/D46165



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

[PATCH] D46024: [clang-format] Add SpaceBeforeCpp11BracedList option.

[Ross Kirsling via Phabricator via cfe-commits]

rkirsling added a comment.

Rule has been published:
https://webkit.org/code-style-guidelines/#spacing-braced-init

Hopefully that suffices for motivation. :)


Repository:
  rC Clang

https://reviews.llvm.org/D46024



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

[PATCH] D46022: [OpenCL] Restrict various keywords in OpenCL C++ mode

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Parse/ParseStmtAsm.cpp:696
+return StmtError();
+  }
+

svenvh wrote:
> rjmccall wrote:
> > svenvh wrote:
> > > rjmccall wrote:
> > > > svenvh wrote:
> > > > > rjmccall wrote:
> > > > > > You might consider parsing the statement normally and then just 
> > > > > > diagnosing after the fact, maybe in Sema.  You'd have to add the 
> > > > > > check in a couple different places, though.
> > > > > Precisely the reason why I did it in the parser, otherwise we'd have 
> > > > > to duplicate the check in multiple places.
> > > > The downside of this is that the parser recovery is probably very bad.  
> > > > But since this is asm, it's not likely to matter too much.
> > > > 
> > > > ...is this *also* really only an OpenCL C++ restriction?  Maybe we 
> > > > should read this one as a general restriction that happens to have been 
> > > > overlooked in the OpenCL C spec.
> > > Yes, `asm` is only explicitly restricted in OpenCL C++.  Not sure if it's 
> > > safe to assume `asm` has been overlooked for OpenCL C though, it's not 
> > > explicitly forbidden so people may be using it.
> > Do you have any contact with the OpenCL committee?  You might want to 
> > collect these issues and present them to them.  They may just not be aware 
> > that `asm` is just as much a feature in C as it is in C++, and it would be 
> > good to understand what the purpose of the `goto` restriction is.
> Yes, we can clarify this with Khronos.  Shall I leave the `asm` and `goto` 
> restrictions out of this patch for now until we have clarification?
That might be best.  It'd be easy enough to apply them later.



Comment at: lib/Sema/SemaDecl.cpp:6380
+  }
+}
   }

This is another thing to check out with Khronos, because `_Thread_local` and 
`__thread` are also available in C.

In this case, I think it's almost certain that they intend the restriction to 
be general, so I'd just proactively apply it in an OpenCL modes.


https://reviews.llvm.org/D46022



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

[PATCH] D45900: CodeGen: Fix invalid bitcast for lifetime.start/end

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Oh, I see, it's not that the lifetime intrinsics don't handle pointers in the 
alloca address space, it's that we might have already promoted them into 
`DefaultAS`.

Do the LLVM uses of lifetime intrinsics actually look through these address 
space casts?  I'm wondering if we might need to change how we emit the 
intrinsics so that they're emitted directly on (bitcasts of) the underlying 
allocas.


https://reviews.llvm.org/D45900



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

[PATCH] D46287: [Driver] Don't add -dwarf-column-info when using -gcodeview on non-msvc targets

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: zturner, rnk, smeenai.
Herald added subscribers: JDevlieghere, aprantl.

-dwarf-column-info is omitted if -gcodeview is specified for msvc targets at 
the moment, but since -gcodeview is an option that can be specified for any 
target, there's little reason to restrict this handling to msvc targets.

This allows getting proper codeview debug info by passing -gcodeview for e.g. 
MinGW targets as well.


Repository:
  rC Clang

https://reviews.llvm.org/D46287

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/codeview-column-info.c


Index: test/Driver/codeview-column-info.c
===
--- test/Driver/codeview-column-info.c
+++ test/Driver/codeview-column-info.c
@@ -6,6 +6,8 @@
 // RUN: FileCheck < %t1 %s
 // RUN: %clangxx -### --target=x86_64-windows-msvc -c -g -gcodeview %s 2> %t2
 // RUN: FileCheck < %t2 %s
+// RUN: %clangxx -### --target=x86_64-windows-gnu -c -g -gcodeview %s 2> %t2
+// RUN: FileCheck < %t2 %s
 // RUN: %clang_cl -### --target=x86_64-windows-msvc /c /Z7 -- %s 2> %t2
 // RUN: FileCheck < %t2 %s
 
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2991,7 +2991,7 @@
   // debuggers don't handle missing end columns well, so it's better not to
   // include any column info.
   if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
-   /*Default=*/!(IsWindowsMSVC && EmitCodeView) &&
+   /*Default=*/!EmitCodeView &&
DebuggerTuning != llvm::DebuggerKind::SCE))
 CmdArgs.push_back("-dwarf-column-info");
 


Index: test/Driver/codeview-column-info.c
===
--- test/Driver/codeview-column-info.c
+++ test/Driver/codeview-column-info.c
@@ -6,6 +6,8 @@
 // RUN: FileCheck < %t1 %s
 // RUN: %clangxx -### --target=x86_64-windows-msvc -c -g -gcodeview %s 2> %t2
 // RUN: FileCheck < %t2 %s
+// RUN: %clangxx -### --target=x86_64-windows-gnu -c -g -gcodeview %s 2> %t2
+// RUN: FileCheck < %t2 %s
 // RUN: %clang_cl -### --target=x86_64-windows-msvc /c /Z7 -- %s 2> %t2
 // RUN: FileCheck < %t2 %s
 
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2991,7 +2991,7 @@
   // debuggers don't handle missing end columns well, so it's better not to
   // include any column info.
   if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
-   /*Default=*/!(IsWindowsMSVC && EmitCodeView) &&
+   /*Default=*/!EmitCodeView &&
DebuggerTuning != llvm::DebuggerKind::SCE))
 CmdArgs.push_back("-dwarf-column-info");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46286: [Driver] Don't warn about unused inputs in config files

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: sepavloff, hans, rnk, hfinkel.

This avoids warnings about unused linker parameters, just like other flags are 
ignored if they're from config files.


Repository:
  rC Clang

https://reviews.llvm.org/D46286

Files:
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-4.cfg
  test/Driver/config-file.c


Index: test/Driver/config-file.c
===
--- test/Driver/config-file.c
+++ test/Driver/config-file.c
@@ -63,6 +63,7 @@
 //
 // RUN: %clang --config %S/Inputs/config-4.cfg -S %s -o /dev/null -v 2>&1 | 
FileCheck %s -check-prefix CHECK-UNUSED
 // CHECK-UNUSED-NOT: argument unused during compilation:
+// CHECK-UNUSED-NOT: 'linker' input unused
 
 
 //--- User directory is searched first.
Index: test/Driver/Inputs/config-4.cfg
===
--- test/Driver/Inputs/config-4.cfg
+++ test/Driver/Inputs/config-4.cfg
@@ -1,2 +1,3 @@
 -L/usr/local/lib
--stdlib=libc++
\ No newline at end of file
+-lfoo
+-stdlib=libc++
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2899,6 +2899,9 @@
 // this compilation, warn the user about it.
 phases::ID InitialPhase = PL[0];
 if (InitialPhase > FinalPhase) {
+  if (InputArg->isClaimed())
+continue;
+
   // Claim here to avoid the more general unused warning.
   InputArg->claim();
 


Index: test/Driver/config-file.c
===
--- test/Driver/config-file.c
+++ test/Driver/config-file.c
@@ -63,6 +63,7 @@
 //
 // RUN: %clang --config %S/Inputs/config-4.cfg -S %s -o /dev/null -v 2>&1 | FileCheck %s -check-prefix CHECK-UNUSED
 // CHECK-UNUSED-NOT: argument unused during compilation:
+// CHECK-UNUSED-NOT: 'linker' input unused
 
 
 //--- User directory is searched first.
Index: test/Driver/Inputs/config-4.cfg
===
--- test/Driver/Inputs/config-4.cfg
+++ test/Driver/Inputs/config-4.cfg
@@ -1,2 +1,3 @@
 -L/usr/local/lib
--stdlib=libc++
\ No newline at end of file
+-lfoo
+-stdlib=libc++
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2899,6 +2899,9 @@
 // this compilation, warn the user about it.
 phases::ID InitialPhase = PL[0];
 if (InitialPhase > FinalPhase) {
+  if (InputArg->isClaimed())
+continue;
+
   // Claim here to avoid the more general unused warning.
   InputArg->claim();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46218: PR37275 packed attribute should not apply to base classes

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

rsmith added a comment.

In https://reviews.llvm.org/D46218#1082834, @probinson wrote:

> In https://reviews.llvm.org/D46218#1081933, @rjmccall wrote:
>
> > I wonder if that was originally just an oversight that got turned into 
> > official policy.  Anyway, if it's the policy, it's what we have to do; LGTM.
>
>
> I think it's actually correct behavior.  Why would an attribute on a derived 
> class be allowed to alter the layout of a base class?  It would mean you 
> can't convert Derived* to Base* safely.


With Clang's old behavior, base classes were treated exactly like fields -- 
their layout was not changed per se, but their minimum alignment was reduced. 
Yes, that means you can't convert `Derived*` to `Base*` safely, but the packed 
attribute means you can't use `` to convert `Class*` to `Member*` 
safely either, so that's not an entirely unique problem. Probably more serious 
is that GCC doesn't permit a `T&` to bind to a packed field, so treating base 
classes as packed would interfere with passing a `Derived` lvalue to a `Base&`, 
which does seem like a much more severe restriction for base classes than for 
fields.


Repository:
  rL LLVM

https://reviews.llvm.org/D46218



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

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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 144617.
JonasToth added a comment.

- fix bad template instantiation


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/ConstCheck.cpp
  clang-tidy/cppcoreguidelines/ConstCheck.h
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/utils/CMakeLists.txt
  clang-tidy/utils/ExprMutationAnalyzer.cpp
  clang-tidy/utils/ExprMutationAnalyzer.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-const.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-const-pointers-as-values.cpp
  test/clang-tidy/cppcoreguidelines-const-values.cpp
  unittests/clang-tidy/CMakeLists.txt
  unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Index: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
===
--- /dev/null
+++ unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -0,0 +1,554 @@
+//===-- ExprMutationAnalyzerTest.cpp - clang-tidy -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "../clang-tidy/utils/ExprMutationAnalyzer.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+using namespace clang::ast_matchers;
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+using ::testing::ResultOf;
+using ::testing::StartsWith;
+using ::testing::Values;
+
+namespace {
+
+using ExprMatcher = internal::Matcher;
+using StmtMatcher = internal::Matcher;
+
+ExprMatcher declRefTo(StringRef Name) {
+  return declRefExpr(to(namedDecl(hasName(Name;
+}
+
+StmtMatcher withEnclosingCompound(ExprMatcher Matcher) {
+  return expr(Matcher, hasAncestor(compoundStmt().bind("stmt"))).bind("expr");
+}
+
+bool isMutated(const SmallVectorImpl , ASTUnit *AST) {
+  const auto *const S = selectFirst("stmt", Results);
+  const auto *const E = selectFirst("expr", Results);
+  return utils::ExprMutationAnalyzer(S, >getASTContext()).isMutated(E);
+}
+
+SmallVector
+mutatedBy(const SmallVectorImpl , ASTUnit *AST) {
+  const auto *const S = selectFirst("stmt", Results);
+  SmallVector Chain;
+  utils::ExprMutationAnalyzer Analyzer(S, >getASTContext());
+  for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
+const Stmt *By = Analyzer.findMutation(E);
+std::string buffer;
+llvm::raw_string_ostream stream(buffer);
+By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
+Chain.push_back(StringRef(stream.str()).trim().str());
+E = dyn_cast(By);
+  }
+  return Chain;
+}
+
+std::string removeSpace(std::string s) {
+  s.erase(std::remove_if(s.begin(), s.end(),
+ [](char c) { return std::isspace(c); }),
+  s.end());
+  return s;
+}
+
+} // namespace
+
+TEST(ExprMutationAnalyzerTest, Trivial) {
+  const auto AST = tooling::buildASTFromCode("void f() { int x; x; }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+}
+
+class AssignmentTest : public ::testing::TestWithParam {};
+
+TEST_P(AssignmentTest, AssignmentModifies) {
+  const std::string ModExpr = "x " + GetParam() + " 10";
+  const auto AST =
+  tooling::buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+}
+
+INSTANTIATE_TEST_CASE_P(AllAssignmentOperators, AssignmentTest,
+Values("=", "+=", "-=", "*=", "/=", "%=", "&=", "|=",
+   "^=", "<<=", ">>="), );
+
+class IncDecTest : public ::testing::TestWithParam {};
+
+TEST_P(IncDecTest, IncDecModifies) {
+  const std::string ModExpr = GetParam();
+  const auto AST =
+  tooling::buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+}
+
+INSTANTIATE_TEST_CASE_P(AllIncDecOperators, IncDecTest,
+Values("++x", "--x", "x++", "x--"), );
+
+TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) {
+  const auto AST = tooling::buildASTFromCode(
+  "void f() { struct Foo { void mf(); }; Foo x; x.mf(); }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I migrated to the new `Decl` interface and adjusted my tests, there are no 
false positives left and I am not aware of more possible code constellation 
that would require testing.
If you have time you could check my tests, maybe you find something I missed.

Otherwise this is LGTM from me, but the reviewers must of course accept it 
(@aaron.ballman, @alexfh)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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

[PATCH] D46281: [clang-doc] Attaching a name to reference data

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 144615.
juliehockett marked 3 inline comments as done.
juliehockett added a comment.

Addressing comments


https://reviews.llvm.org/D46281

Files:
  clang-doc/BitcodeWriter.cpp
  clang-doc/BitcodeWriter.h
  clang-doc/Representation.h
  clang-doc/Serialize.cpp
  test/clang-doc/mapper-class-in-class.cpp
  test/clang-doc/mapper-class-in-function.cpp
  test/clang-doc/mapper-class.cpp
  test/clang-doc/mapper-comments.cpp
  test/clang-doc/mapper-enum.cpp
  test/clang-doc/mapper-function.cpp
  test/clang-doc/mapper-method.cpp
  test/clang-doc/mapper-namespace.cpp
  test/clang-doc/mapper-struct.cpp
  test/clang-doc/mapper-union.cpp

Index: test/clang-doc/mapper-union.cpp
===
--- test/clang-doc/mapper-union.cpp
+++ test/clang-doc/mapper-union.cpp
@@ -9,21 +9,27 @@
 
 // CHECK: 
 // CHECK-NEXT: 
-  // CHECK-NEXT: 
+  // CHECK-NEXT: 
 // CHECK-NEXT: 
 // CHECK-NEXT: 
   // CHECK-NEXT: 
   // CHECK-NEXT:  blob data = 'D'
-  // CHECK-NEXT:  blob data = '{{.*}}'
-  // CHECK-NEXT: 
-  // CHECK-NEXT: 
-// CHECK-NEXT:  blob data = 'int'
-// CHECK-NEXT:  blob data = 'D::X'
-// CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT: 
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'int'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  blob data = 'D::X'
+// CHECK-NEXT: 
   // CHECK-NEXT: 
-  // CHECK-NEXT: 
-// CHECK-NEXT:  blob data = 'int'
-// CHECK-NEXT:  blob data = 'D::Y'
-// CHECK-NEXT: 
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'int'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  blob data = 'D::Y'
+// CHECK-NEXT: 
   // CHECK-NEXT: 
 // CHECK-NEXT: 
Index: test/clang-doc/mapper-struct.cpp
===
--- test/clang-doc/mapper-struct.cpp
+++ test/clang-doc/mapper-struct.cpp
@@ -9,15 +9,18 @@
 
 // CHECK: 
 // CHECK-NEXT: 
-  // CHECK-NEXT: 
+  // CHECK-NEXT: 
 // CHECK-NEXT: 
 // CHECK-NEXT: 
   // CHECK-NEXT: 
   // CHECK-NEXT:  blob data = 'C'
-  // CHECK-NEXT:  blob data = '{{.*}}'
-  // CHECK-NEXT: 
-// CHECK-NEXT:  blob data = 'int'
-// CHECK-NEXT:  blob data = 'C::i'
-// CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'int'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  blob data = 'C::i'
+// CHECK-NEXT: 
   // CHECK-NEXT: 
 // CHECK-NEXT: 
Index: test/clang-doc/mapper-namespace.cpp
===
--- test/clang-doc/mapper-namespace.cpp
+++ test/clang-doc/mapper-namespace.cpp
@@ -9,7 +9,7 @@
 
 // CHECK: 
 // CHECK-NEXT: 
-  // CHECK-NEXT: 
+  // CHECK-NEXT: 
 // CHECK-NEXT: 
 // CHECK-NEXT: 
   // CHECK-NEXT: 
Index: test/clang-doc/mapper-method.cpp
===
--- test/clang-doc/mapper-method.cpp
+++ test/clang-doc/mapper-method.cpp
@@ -13,31 +13,47 @@
 
 // CHECK-G: 
 // CHECK-G-NEXT: 
-  // CHECK-G-NEXT: 
+  // CHECK-G-NEXT: 
 // CHECK-G-NEXT: 
 // CHECK-G-NEXT: 
   // CHECK-G-NEXT: 
   // CHECK-G-NEXT:  blob data = 'G'
-  // CHECK-G-NEXT:  blob data = '{{.*}}'
-  // CHECK-G-NEXT: 
+  // CHECK-G-NEXT:  blob data = '{{.*}}'
+  // CHECK-G-NEXT: 
 // CHECK-G-NEXT: 
 
 // CHECK-G-F: 
 // CHECK-G-F-NEXT: 
-  // CHECK-G-F-NEXT: 
+  // CHECK-G-F-NEXT: 
 // CHECK-G-F-NEXT: 
 // CHECK-G-F-NEXT: 
   // CHECK-G-F-NEXT: 
   // CHECK-G-F-NEXT:  blob data = 'Method'
-  // CHECK-G-F-NEXT:  blob data = '4202E8BF0ECB12AE354C8499C52725B0EE30AED5'
-  // CHECK-G-F-NEXT: 
-  // CHECK-G-F-NEXT:  blob data = '{{.*}}'
-  // CHECK-G-F-NEXT:  blob data = '4202E8BF0ECB12AE354C8499C52725B0EE30AED5'
-  // CHECK-G-F-NEXT: 
-// CHECK-G-F-NEXT:  blob data = 'int'
+  // CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT:  blob data = 'G'
+// CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+  // CHECK-G-F-NEXT: 
+  // CHECK-G-F-NEXT: 
+  // CHECK-G-F-NEXT:  blob data = '{{.*}}'
+  // CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT:  blob data = 'G'
+// CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+  // CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+  // CHECK-G-F-NEXT:  blob data = 'int'
+  // CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
   // CHECK-G-F-NEXT: 
-  // CHECK-G-F-NEXT: 
-// CHECK-G-F-NEXT:  blob data = 'int'
-// CHECK-G-F-NEXT:  blob data = 'param'
+  // CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+  // CHECK-G-F-NEXT:  blob data = 'int'
+  // CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT:  blob data = 'param'
   // CHECK-G-F-NEXT: 
 // CHECK-G-F-NEXT: 
Index: test/clang-doc/mapper-function.cpp
===
--- test/clang-doc/mapper-function.cpp
+++ 

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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 144616.
JonasToth added a comment.

- migrate to Decl interface
- add template metaprogramming test


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/ConstCheck.cpp
  clang-tidy/cppcoreguidelines/ConstCheck.h
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/utils/CMakeLists.txt
  clang-tidy/utils/ExprMutationAnalyzer.cpp
  clang-tidy/utils/ExprMutationAnalyzer.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-const.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-const-pointers-as-values.cpp
  test/clang-tidy/cppcoreguidelines-const-values.cpp
  unittests/clang-tidy/CMakeLists.txt
  unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Index: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
===
--- /dev/null
+++ unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -0,0 +1,554 @@
+//===-- ExprMutationAnalyzerTest.cpp - clang-tidy -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "../clang-tidy/utils/ExprMutationAnalyzer.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+using namespace clang::ast_matchers;
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+using ::testing::ResultOf;
+using ::testing::StartsWith;
+using ::testing::Values;
+
+namespace {
+
+using ExprMatcher = internal::Matcher;
+using StmtMatcher = internal::Matcher;
+
+ExprMatcher declRefTo(StringRef Name) {
+  return declRefExpr(to(namedDecl(hasName(Name;
+}
+
+StmtMatcher withEnclosingCompound(ExprMatcher Matcher) {
+  return expr(Matcher, hasAncestor(compoundStmt().bind("stmt"))).bind("expr");
+}
+
+bool isMutated(const SmallVectorImpl , ASTUnit *AST) {
+  const auto *const S = selectFirst("stmt", Results);
+  const auto *const E = selectFirst("expr", Results);
+  return utils::ExprMutationAnalyzer(S, >getASTContext()).isMutated(E);
+}
+
+SmallVector
+mutatedBy(const SmallVectorImpl , ASTUnit *AST) {
+  const auto *const S = selectFirst("stmt", Results);
+  SmallVector Chain;
+  utils::ExprMutationAnalyzer Analyzer(S, >getASTContext());
+  for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
+const Stmt *By = Analyzer.findMutation(E);
+std::string buffer;
+llvm::raw_string_ostream stream(buffer);
+By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
+Chain.push_back(StringRef(stream.str()).trim().str());
+E = dyn_cast(By);
+  }
+  return Chain;
+}
+
+std::string removeSpace(std::string s) {
+  s.erase(std::remove_if(s.begin(), s.end(),
+ [](char c) { return std::isspace(c); }),
+  s.end());
+  return s;
+}
+
+} // namespace
+
+TEST(ExprMutationAnalyzerTest, Trivial) {
+  const auto AST = tooling::buildASTFromCode("void f() { int x; x; }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+}
+
+class AssignmentTest : public ::testing::TestWithParam {};
+
+TEST_P(AssignmentTest, AssignmentModifies) {
+  const std::string ModExpr = "x " + GetParam() + " 10";
+  const auto AST =
+  tooling::buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+}
+
+INSTANTIATE_TEST_CASE_P(AllAssignmentOperators, AssignmentTest,
+Values("=", "+=", "-=", "*=", "/=", "%=", "&=", "|=",
+   "^=", "<<=", ">>="), );
+
+class IncDecTest : public ::testing::TestWithParam {};
+
+TEST_P(IncDecTest, IncDecModifies) {
+  const std::string ModExpr = GetParam();
+  const auto AST =
+  tooling::buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+}
+
+INSTANTIATE_TEST_CASE_P(AllIncDecOperators, IncDecTest,
+Values("++x", "--x", "x++", "x--"), );
+
+TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) {
+  const auto AST = tooling::buildASTFromCode(
+  "void f() { struct Foo { void mf(); }; Foo x; x.mf(); }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), 

r331218 - [MC] Change AsmParser to leverage Assembler during evaluation

[Nirav Dave via cfe-commits]

Author: niravd
Date: Mon Apr 30 12:22:40 2018
New Revision: 331218

URL: http://llvm.org/viewvc/llvm-project?rev=331218=rev
Log:
[MC] Change AsmParser to leverage Assembler during evaluation

Teach AsmParser to check with Assembler for when evaluating constant
expressions.  This improves the handing of preprocessor expressions
that must be resolved at parse time. This idiom can be found as
assembling-time assertion checks in source-level assemblers. Note that
this relies on the MCStreamer to keep sufficient tabs on Section /
Fragment information which the MCAsmStreamer does not. As a result the
textual output may fail where the equivalent object generation would
pass. This can most easily be resolved by folding the MCAsmStreamer
and MCObjectStreamer together which is planned for in a separate
patch.

Currently, this feature is only enabled for assembly input, keeping IR
compilation consistent between assembly and object generation.

Reviewers: echristo, rnk, probinson, espindola, peter.smith

Reviewed By: peter.smith

Subscribers: eraman, peter.smith, arichardson, jyknight, hiraditya, llvm-commits

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

Added:
cfe/trunk/test/CodeGen/asm-parser-info.S
Modified:
cfe/trunk/tools/driver/cc1as_main.cpp

Added: cfe/trunk/test/CodeGen/asm-parser-info.S
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/asm-parser-info.S?rev=331218=auto
==
--- cfe/trunk/test/CodeGen/asm-parser-info.S (added)
+++ cfe/trunk/test/CodeGen/asm-parser-info.S Mon Apr 30 12:22:40 2018
@@ -0,0 +1,12 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang --target=x86_64-unknown-linux-gnu -c %s -o /dev/null
+
+// Check that cc1as can use assembler info in object generation.
+.data
+   
+foo:
+.if . - foo == 0
+.byte 0xaa
+.else
+.byte 0x00
+.endif

Modified: cfe/trunk/tools/driver/cc1as_main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1as_main.cpp?rev=331218=331217=331218=diff
==
--- cfe/trunk/tools/driver/cc1as_main.cpp (original)
+++ cfe/trunk/tools/driver/cc1as_main.cpp Mon Apr 30 12:22:40 2018
@@ -435,6 +435,9 @@ static bool ExecuteAssembler(AssemblerIn
 Str.get()->InitSections(Opts.NoExecStack);
   }
 
+  // Assembly to object compilation should leverage assembly info.
+  Str->setUseAssemblerInfoForParsing(true);
+
   bool Failed = false;
 
   std::unique_ptr Parser(


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

[PATCH] D45164: [MC] Change AsmParser to leverage Assembler during evaluation

[Nirav Dave via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC331218: [MC] Change AsmParser to leverage Assembler during 
evaluation (authored by niravd, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45164?vs=143128=144613#toc

Repository:
  rC Clang

https://reviews.llvm.org/D45164

Files:
  test/CodeGen/asm-parser-info.S
  tools/driver/cc1as_main.cpp


Index: test/CodeGen/asm-parser-info.S
===
--- test/CodeGen/asm-parser-info.S
+++ test/CodeGen/asm-parser-info.S
@@ -0,0 +1,12 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang --target=x86_64-unknown-linux-gnu -c %s -o /dev/null
+
+// Check that cc1as can use assembler info in object generation.
+.data
+   
+foo:
+.if . - foo == 0
+.byte 0xaa
+.else
+.byte 0x00
+.endif
Index: tools/driver/cc1as_main.cpp
===
--- tools/driver/cc1as_main.cpp
+++ tools/driver/cc1as_main.cpp
@@ -435,6 +435,9 @@
 Str.get()->InitSections(Opts.NoExecStack);
   }
 
+  // Assembly to object compilation should leverage assembly info.
+  Str->setUseAssemblerInfoForParsing(true);
+
   bool Failed = false;
 
   std::unique_ptr Parser(


Index: test/CodeGen/asm-parser-info.S
===
--- test/CodeGen/asm-parser-info.S
+++ test/CodeGen/asm-parser-info.S
@@ -0,0 +1,12 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang --target=x86_64-unknown-linux-gnu -c %s -o /dev/null
+
+// Check that cc1as can use assembler info in object generation.
+.data
+	
+foo:
+.if . - foo == 0
+.byte 0xaa
+.else
+.byte 0x00
+.endif
Index: tools/driver/cc1as_main.cpp
===
--- tools/driver/cc1as_main.cpp
+++ tools/driver/cc1as_main.cpp
@@ -435,6 +435,9 @@
 Str.get()->InitSections(Opts.NoExecStack);
   }
 
+  // Assembly to object compilation should leverage assembly info.
+  Str->setUseAssemblerInfoForParsing(true);
+
   bool Failed = false;
 
   std::unique_ptr Parser(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46281: [clang-doc] Attaching a name to reference data

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-doc/BitcodeWriter.cpp:275
 
+static SymbolID SID;
 void ClangDocBitcodeWriter::emitRecord(const SymbolID , RecordId ID) {

1. I'm not seeing where this is being changed?
2. This looks like some abstraction is missing. Maybe at least put it as 
private member of `ClangDocBitcodeWriter`?



Comment at: clang-doc/BitcodeWriter.h:34
 // BitCodeConstants, though they can be added without breaking it.
 static const unsigned VersionNumber = 1;
 

Looking at the changes, may want to bump the version.



Comment at: clang-doc/BitcodeWriter.h:64
   BI_FIRST = BI_VERSION_BLOCK_ID,
-  BI_LAST = BI_COMMENT_BLOCK_ID
+  BI_LAST = BI_REFERENCE_BLOCK_ID
 };

General observation: you could keep `BI_LAST` element as the last one in the 
enum (just like the rest,
without specifying it's value), and use it just like the `end()`, i.e. `v < 
BI_LAST` instead of `v <= BI_LAST`.


https://reviews.llvm.org/D46281



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

r331216 - AMDGPU: Add Vega12 and Vega20

[Matt Arsenault via cfe-commits]

Author: arsenm
Date: Mon Apr 30 12:08:27 2018
New Revision: 331216

URL: http://llvm.org/viewvc/llvm-project?rev=331216=rev
Log:
AMDGPU: Add Vega12 and Vega20

Changes by
  Matt Arsenault
  Konstantin Zhuravlyov

Added:
cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl
cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-dl-insts-err.cl
cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn-dl-insts.cl
Modified:
cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
cfe/trunk/lib/Basic/Targets/AMDGPU.h
cfe/trunk/test/Driver/amdgpu-macros.cl
cfe/trunk/test/Driver/amdgpu-mcpu.cl

Modified: cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def?rev=331216=331215=331216=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def Mon Apr 30 12:08:27 2018
@@ -121,6 +121,18 @@ TARGET_BUILTIN(__builtin_amdgcn_mov_dpp,
 TARGET_BUILTIN(__builtin_amdgcn_fmed3h, "", "nc", "gfx9-insts")
 
 
//===--===//
+// Deep learning builtins.
+//===--===//
+
+TARGET_BUILTIN(__builtin_amdgcn_fdot2, "fV2hV2hf", "nc", "dl-insts")
+TARGET_BUILTIN(__builtin_amdgcn_sdot2, "SiV2SsV2SsSi", "nc", "dl-insts")
+TARGET_BUILTIN(__builtin_amdgcn_udot2, "UiV2UsV2UsUi", "nc", "dl-insts")
+TARGET_BUILTIN(__builtin_amdgcn_sdot4, "SiSiSiSi", "nc", "dl-insts")
+TARGET_BUILTIN(__builtin_amdgcn_udot4, "UiUiUiUi", "nc", "dl-insts")
+TARGET_BUILTIN(__builtin_amdgcn_sdot8, "SiSiSiSi", "nc", "dl-insts")
+TARGET_BUILTIN(__builtin_amdgcn_udot8, "UiUiUiUi", "nc", "dl-insts")
+
+//===--===//
 // Special builtins.
 
//===--===//
 BUILTIN(__builtin_amdgcn_read_exec, "LUi", "nc")

Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AMDGPU.cpp?rev=331216=331215=331216=diff
==
--- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp Mon Apr 30 12:08:27 2018
@@ -133,6 +133,10 @@ bool AMDGPUTargetInfo::initFeatureMap(
   CPU = "gfx600";
 
 switch (parseAMDGCNName(CPU).Kind) {
+case GK_GFX906:
+  Features["dl-insts"] = true;
+  LLVM_FALLTHROUGH;
+case GK_GFX904:
 case GK_GFX902:
 case GK_GFX900:
   Features["gfx9-insts"] = true;

Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AMDGPU.h?rev=331216=331215=331216=diff
==
--- cfe/trunk/lib/Basic/Targets/AMDGPU.h (original)
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.h Mon Apr 30 12:08:27 2018
@@ -78,9 +78,11 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTarg
 GK_GFX810,
 GK_GFX900,
 GK_GFX902,
+GK_GFX904,
+GK_GFX906,
 
 GK_AMDGCN_FIRST = GK_GFX600,
-GK_AMDGCN_LAST = GK_GFX902,
+GK_AMDGCN_LAST = GK_GFX906,
   };
 
   struct GPUInfo {
@@ -127,7 +129,7 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTarg
 {{"cayman"},  {"cayman"},  GK_CAYMAN,  true,  false, false, false, false},
 {{"turks"},   {"turks"},   GK_TURKS,   false, false, false, false, false},
   };
-  static constexpr GPUInfo AMDGCNGPUs[30] = {
+  static constexpr GPUInfo AMDGCNGPUs[32] = {
   // Name   CanonicalKindHas   HasHasHas   Has
   //Name FMAF  Fast   LDEXPF FP64  Fast
   //   FMAFFMA
@@ -161,6 +163,8 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTarg
 {{"stoney"},{"gfx810"},  GK_GFX810,  true, false, true,  true, true},
 {{"gfx900"},{"gfx900"},  GK_GFX900,  true, true,  true,  true, true},
 {{"gfx902"},{"gfx902"},  GK_GFX900,  true, true,  true,  true, true},
+{{"gfx904"},{"gfx904"},  GK_GFX904,  true, true,  true,  true, true},
+{{"gfx906"},{"gfx906"},  GK_GFX906,  true, true,  true,  true, true},
   };
 
   static GPUInfo parseR600Name(StringRef Name);

Added: cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl?rev=331216=auto
==
--- cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl (added)
+++ cfe/trunk/test/CodeGenOpenCL/amdgpu-features.cl Mon Apr 30 12:08:27 2018
@@ -0,0 +1,12 @@
+// REQUIRES: amdgpu-registered-target
+
+// Check that appropriate features are defined for every supported AMDGPU
+// "-target" and "-mcpu" 

[PATCH] D46281: [clang-doc] Attaching a name to reference data

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: sammccall, jakehehrlich, lebedev.ri.
juliehockett added a project: clang-tools-extra.

This adds the name of the referenced decl, in addition to its USR, to the saved 
data, so that the backend can look at an info in isolation and still be able to 
construct a human-readable name for it.


https://reviews.llvm.org/D46281

Files:
  clang-doc/BitcodeWriter.cpp
  clang-doc/BitcodeWriter.h
  clang-doc/Representation.h
  clang-doc/Serialize.cpp
  test/clang-doc/mapper-class-in-class.cpp
  test/clang-doc/mapper-class-in-function.cpp
  test/clang-doc/mapper-class.cpp
  test/clang-doc/mapper-comments.cpp
  test/clang-doc/mapper-enum.cpp
  test/clang-doc/mapper-function.cpp
  test/clang-doc/mapper-method.cpp
  test/clang-doc/mapper-struct.cpp
  test/clang-doc/mapper-union.cpp

Index: test/clang-doc/mapper-union.cpp
===
--- test/clang-doc/mapper-union.cpp
+++ test/clang-doc/mapper-union.cpp
@@ -14,16 +14,22 @@
 // CHECK-NEXT: 
   // CHECK-NEXT: 
   // CHECK-NEXT:  blob data = 'D'
-  // CHECK-NEXT:  blob data = '{{.*}}'
-  // CHECK-NEXT: 
-  // CHECK-NEXT: 
-// CHECK-NEXT:  blob data = 'int'
-// CHECK-NEXT:  blob data = 'D::X'
-// CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT: 
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'int'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  blob data = 'D::X'
+// CHECK-NEXT: 
   // CHECK-NEXT: 
-  // CHECK-NEXT: 
-// CHECK-NEXT:  blob data = 'int'
-// CHECK-NEXT:  blob data = 'D::Y'
-// CHECK-NEXT: 
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'int'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  blob data = 'D::Y'
+// CHECK-NEXT: 
   // CHECK-NEXT: 
 // CHECK-NEXT: 
Index: test/clang-doc/mapper-struct.cpp
===
--- test/clang-doc/mapper-struct.cpp
+++ test/clang-doc/mapper-struct.cpp
@@ -14,10 +14,13 @@
 // CHECK-NEXT: 
   // CHECK-NEXT: 
   // CHECK-NEXT:  blob data = 'C'
-  // CHECK-NEXT:  blob data = '{{.*}}'
-  // CHECK-NEXT: 
-// CHECK-NEXT:  blob data = 'int'
-// CHECK-NEXT:  blob data = 'C::i'
-// CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'int'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  blob data = 'C::i'
+// CHECK-NEXT: 
   // CHECK-NEXT: 
 // CHECK-NEXT: 
Index: test/clang-doc/mapper-method.cpp
===
--- test/clang-doc/mapper-method.cpp
+++ test/clang-doc/mapper-method.cpp
@@ -18,8 +18,8 @@
 // CHECK-G-NEXT: 
   // CHECK-G-NEXT: 
   // CHECK-G-NEXT:  blob data = 'G'
-  // CHECK-G-NEXT:  blob data = '{{.*}}'
-  // CHECK-G-NEXT: 
+  // CHECK-G-NEXT:  blob data = '{{.*}}'
+  // CHECK-G-NEXT: 
 // CHECK-G-NEXT: 
 
 // CHECK-G-F: 
@@ -29,15 +29,31 @@
 // CHECK-G-F-NEXT: 
   // CHECK-G-F-NEXT: 
   // CHECK-G-F-NEXT:  blob data = 'Method'
-  // CHECK-G-F-NEXT:  blob data = '4202E8BF0ECB12AE354C8499C52725B0EE30AED5'
-  // CHECK-G-F-NEXT: 
-  // CHECK-G-F-NEXT:  blob data = '{{.*}}'
-  // CHECK-G-F-NEXT:  blob data = '4202E8BF0ECB12AE354C8499C52725B0EE30AED5'
-  // CHECK-G-F-NEXT: 
-// CHECK-G-F-NEXT:  blob data = 'int'
+  // CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT:  blob data = 'G'
+// CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+  // CHECK-G-F-NEXT: 
+  // CHECK-G-F-NEXT: 
+  // CHECK-G-F-NEXT:  blob data = '{{.*}}'
+  // CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT:  blob data = 'G'
+// CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+  // CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+  // CHECK-G-F-NEXT:  blob data = 'int'
+  // CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
   // CHECK-G-F-NEXT: 
-  // CHECK-G-F-NEXT: 
-// CHECK-G-F-NEXT:  blob data = 'int'
-// CHECK-G-F-NEXT:  blob data = 'param'
+  // CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+  // CHECK-G-F-NEXT:  blob data = 'int'
+  // CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT: 
+// CHECK-G-F-NEXT:  blob data = 'param'
   // CHECK-G-F-NEXT: 
 // CHECK-G-F-NEXT: 
Index: test/clang-doc/mapper-function.cpp
===
--- test/clang-doc/mapper-function.cpp
+++ test/clang-doc/mapper-function.cpp
@@ -14,12 +14,18 @@
 // CHECK-NEXT: 
   // CHECK-NEXT: 
   // CHECK-NEXT:  blob data = 'F'
-  // CHECK-NEXT:  blob data = '{{.*}}'
-  // CHECK-NEXT: 
-// CHECK-NEXT:  blob data = 'int'
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'int'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
   // CHECK-NEXT: 
-  // CHECK-NEXT: 
-// CHECK-NEXT:  blob data = 'int'
-// CHECK-NEXT:  blob data = 'param'
+  

r331214 - clang-cl: Expose -fmerge-all-constants

[Hans Wennborg via cfe-commits]

Author: hans
Date: Mon Apr 30 12:04:04 2018
New Revision: 331214

URL: http://llvm.org/viewvc/llvm-project?rev=331214=rev
Log:
clang-cl: Expose -fmerge-all-constants

Now that constant merging is off by default, we'd like a way to enable
it on Windows.

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/test/Driver/cl-options.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=331214=331213=331214=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Apr 30 12:04:04 2018
@@ -1176,7 +1176,7 @@ def fthinlto_index_EQ : Joined<["-"], "f
 def fmacro_backtrace_limit_EQ : Joined<["-"], "fmacro-backtrace-limit=">,
 Group, Flags<[DriverOption, 
CoreOption]>;
 def fmerge_all_constants : Flag<["-"], "fmerge-all-constants">, Group,
-  Flags<[CC1Option]>, HelpText<"Allow merging of constants">;
+  Flags<[CC1Option, CoreOption]>, HelpText<"Allow merging of constants">;
 def fmessage_length_EQ : Joined<["-"], "fmessage-length=">, Group;
 def fms_extensions : Flag<["-"], "fms-extensions">, Group, 
Flags<[CC1Option, CoreOption]>,
   HelpText<"Accept some non-standard constructs supported by the Microsoft 
compiler">;

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=331214=331213=331214=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Mon Apr 30 12:04:04 2018
@@ -590,6 +590,7 @@
 // RUN: -fstandalone-debug \
 // RUN: -flimit-debug-info \
 // RUN: -flto \
+// RUN: -fmerge-all-constants \
 // RUN: --version \
 // RUN: -Werror /Zs -- %s 2>&1
 


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

[PATCH] D45931: [ASTMatchers] Don't garble the profiling output when multiple TU's are processed

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Thank you for looking at this.

In https://reviews.llvm.org/D45931#1083184, @alexfh wrote:

> From a user's perspective I'd probably prefer a different behavior of checks 
> profiling with multiple translation units: per-file table after each file and 
> an aggregate table at the end.


Is this a review note, or a general observation?

I'm sure it could be done, i'm just not //too// sure how useful it would be, 
since it seems no one before now even noticed that timing with multiple TU's 
was 'broken'.

> An independent improvement could be to support TSV/CSV output and/or dumping 
> to a file to spare the user from parsing the tables out of the stdout/stderr.

Yes, and a script to merge those CSV's, would be nice.


Repository:
  rC Clang

https://reviews.llvm.org/D45931



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

[PATCH] D45292: [Driver] Obey computed sysroot when finding libc++ headers.

[Dan Albert via Phabricator via cfe-commits]

danalbert added a comment.

Ping.


Repository:
  rC Clang

https://reviews.llvm.org/D45292



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

[PATCH] D45291: [Driver] Infer Android sysroot location.

[Dan Albert via Phabricator via cfe-commits]

danalbert added a comment.

Ping.


Repository:
  rC Clang

https://reviews.llvm.org/D45291



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

[PATCH] D45931: [ASTMatchers] Don't garble the profiling output when multiple TU's are processed

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

From a user's perspective I'd probably prefer a different behavior of checks 
profiling with multiple translation units: per-file table after each file and 
an aggregate table at the end. An independent improvement could be to support 
TSV/CSV output and/or dumping to a file to spare the user from parsing the 
tables out of the stdout/stderr.


Repository:
  rC Clang

https://reviews.llvm.org/D45931



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

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D45927#1083051, @zinovy.nis wrote:

> > I think, it's 13, if you choose to remove stars, and 17 otherwise. The 
> > difference is excessive spaces vs. required ones. Implementing proper logic 
> > may be involved, but we can simplify it to something like "count all 
> > non-space characters and a single space between words, but don't count 
> > spaces around punctuation":
>
> Isn't it a business of clang-format to determine the number of spaces between 
> lexemes? IMO the solutuion you've provided for `GetTypeNameLength` is enough. 
> Considering punctuation here is overkill :-)


The algorithm in your patch makes decisions based on the assumption that there 
may be no spaces at all, which is definitely wrong. I'm proposing an algorithm 
that would at least handle things like `unsigned char` correctly.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45927



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

r331211 - [OPENMP] Do not emit warning about non-declared target function params.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Apr 30 11:28:08 2018
New Revision: 331211

URL: http://llvm.org/viewvc/llvm-project?rev=331211=rev
Log:
[OPENMP] Do not emit warning about non-declared target function params.

We should not emit warning that the parameters are not marked as declare
target, these declaration are local and cannot be marked as declare
target.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/declare_target_messages.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=331211=331210=331211=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Mon Apr 30 11:28:08 2018
@@ -12962,7 +12962,7 @@ static void checkDeclInTargetContext(Sou
   if (!LD)
 LD = D;
   if (LD && !LD->hasAttr() &&
-  (isa(LD) || isa(LD))) {
+  ((isa(LD) && !isa(LD)) || isa(LD))) {
 // Outlined declaration is not declared target.
 if (LD->isOutOfLine()) {
   SemaRef.Diag(LD->getLocation(), diag::warn_omp_not_in_target_context);

Modified: cfe/trunk/test/OpenMP/declare_target_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_target_messages.cpp?rev=331211=331210=331211=diff
==
--- cfe/trunk/test/OpenMP/declare_target_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_target_messages.cpp Mon Apr 30 11:28:08 2018
@@ -88,7 +88,7 @@ int C::method1() {
   return 0;
 }
 
-void foo() {
+void foo(int p) {
   a = 0; // expected-error {{threadprivate variables cannot be used in target 
constructs}}
   b = 0; // expected-note {{used here}}
   t = 1; // expected-error {{threadprivate variables cannot be used in target 
constructs}}
@@ -96,7 +96,7 @@ void foo() {
   VC object1;
   g = object.method();
   g += object.method1();
-  g += object1.method();
+  g += object1.method() + p;
   f();
   c(); // expected-note {{used here}}
 }
@@ -119,7 +119,7 @@ int main (int argc, char **argv) {
 #pragma omp declare target // expected-error {{unexpected OpenMP directive 
'#pragma omp declare target'}}
   int v;
 #pragma omp end declare target // expected-error {{unexpected OpenMP directive 
'#pragma omp end declare target'}}
-  foo();
+  foo(v);
   return (0);
 }
 


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

[PATCH] D45900: CodeGen: Fix invalid bitcast for lifetime.start/end

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 144600.
yaxunl added a comment.

Call Builder.CreateLifetimeStart/End.

I cannot replace CodeGenFunction::EmitLifetimeStart with 
Builder.CreateLifetimeStart since we still need to check 
ShouldEmitLifetimeMarkers and do the address space cast.


https://reviews.llvm.org/D45900

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGenCXX/amdgcn_declspec_get.cpp


Index: test/CodeGenCXX/amdgcn_declspec_get.cpp
===
--- /dev/null
+++ test/CodeGenCXX/amdgcn_declspec_get.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm -O3 -fdeclspec \
+// RUN: -disable-llvm-passes -o - %s | FileCheck %s
+
+int get_x();
+
+struct A {
+   __declspec(property(get = _get_x)) int x;
+   static int _get_x(void) {
+ return get_x();
+   };
+};
+
+extern const A a;
+
+// CHECK-LABEL: define void @_Z4testv()
+// CHECK:  %i = alloca i32, align 4, addrspace(5)
+// CHECK:  %[[ii:.*]] = addrspacecast i32 addrspace(5)* %i to i32*
+// CHECK:  %[[cast1:.*]] = addrspacecast i32* %[[ii]] to i8 addrspace(5)*
+// CHECK:  call void @llvm.lifetime.start.p5i8(i64 4, i8 addrspace(5)* 
%[[cast1]])
+// CHECK:  %call = call i32 @_ZN1A6_get_xEv()
+// CHECK:  store i32 %call, i32* %[[ii]]
+// CHECK:  %[[cast2:.*]] = addrspacecast i32* %[[ii]] to i8 addrspace(5)*
+// CHECK:  call void @llvm.lifetime.end.p5i8(i64 4, i8 addrspace(5)* 
%[[cast2]])
+void test()
+{
+  int i = a.x;
+}
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2084,6 +2084,9 @@
   /// Emit a cast to void* in the appropriate address space.
   llvm::Value *EmitCastToVoidPtr(llvm::Value *value);
 
+  /// Emit a cast to void* in alloca address space.
+  llvm::Value *EmitCastToVoidPtrInAllocaAddrSpace(llvm::Value *V);
+
   /// EvaluateExprAsBool - Perform the usual unary conversions on the specified
   /// expression and compare the result against zero, returning an Int1Ty 
value.
   llvm::Value *EvaluateExprAsBool(const Expr *E);
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -59,6 +59,18 @@
   return Builder.CreateBitCast(value, destType);
 }
 
+llvm::Value *
+CodeGenFunction::EmitCastToVoidPtrInAllocaAddrSpace(llvm::Value *V) {
+  if (V->getType()->getPointerAddressSpace() !=
+  CGM.getDataLayout().getAllocaAddrSpace()) {
+return getTargetHooks().performAddrSpaceCast(
+*this, V, getASTAllocaAddressSpace(), LangAS::Default, AllocaInt8PtrTy,
+/*non-null*/ true);
+  } else {
+return Builder.CreateBitCast(V, AllocaInt8PtrTy);
+  }
+}
+
 /// CreateTempAlloca - This creates a alloca and inserts it into the entry
 /// block.
 Address CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, CharUnits Align,
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -964,20 +964,15 @@
 llvm::Value *Addr) {
   if (!ShouldEmitLifetimeMarkers)
 return nullptr;
-
-  llvm::Value *SizeV = llvm::ConstantInt::get(Int64Ty, Size);
-  Addr = Builder.CreateBitCast(Addr, AllocaInt8PtrTy);
-  llvm::CallInst *C =
-  Builder.CreateCall(CGM.getLLVMLifetimeStartFn(), {SizeV, Addr});
-  C->setDoesNotThrow();
+  Addr = EmitCastToVoidPtrInAllocaAddrSpace(Addr);
+  auto *SizeV = llvm::ConstantInt::get(Int64Ty, Size);
+  Builder.CreateLifetimeStart(Addr, SizeV);
   return SizeV;
 }
 
 void CodeGenFunction::EmitLifetimeEnd(llvm::Value *Size, llvm::Value *Addr) {
-  Addr = Builder.CreateBitCast(Addr, AllocaInt8PtrTy);
-  llvm::CallInst *C =
-  Builder.CreateCall(CGM.getLLVMLifetimeEndFn(), {Size, Addr});
-  C->setDoesNotThrow();
+  Addr = EmitCastToVoidPtrInAllocaAddrSpace(Addr);
+  Builder.CreateLifetimeEnd(Addr, llvm::cast(Size));
 }
 
 void CodeGenFunction::EmitAndRegisterVariableArrayDimensions(


Index: test/CodeGenCXX/amdgcn_declspec_get.cpp
===
--- /dev/null
+++ test/CodeGenCXX/amdgcn_declspec_get.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm -O3 -fdeclspec \
+// RUN: -disable-llvm-passes -o - %s | FileCheck %s
+
+int get_x();
+
+struct A {
+   __declspec(property(get = _get_x)) int x;
+   static int _get_x(void) {
+ return get_x();
+   };
+};
+
+extern const A a;
+
+// CHECK-LABEL: define void @_Z4testv()
+// CHECK:  %i = alloca i32, align 4, addrspace(5)
+// CHECK:  %[[ii:.*]] = addrspacecast i32 addrspace(5)* %i to i32*
+// CHECK:  %[[cast1:.*]] = addrspacecast i32* %[[ii]] to i8 addrspace(5)*
+// CHECK:  call void @llvm.lifetime.start.p5i8(i64 4, i8 addrspace(5)* %[[cast1]])
+// CHECK:  %call = call i32 @_ZN1A6_get_xEv()
+// 

[PATCH] D46236: [Driver, CodeGen] rename options to disable an FP cast optimization

[Sanjay Patel via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL331209: [Driver, CodeGen] rename options to disable an FP 
cast optimization (authored by spatel, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D46236?vs=144551=144599#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D46236

Files:
  cfe/trunk/docs/ReleaseNotes.rst
  cfe/trunk/docs/UsersManual.rst
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/lib/CodeGen/CGCall.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/no-junk-ftrunc.c
  cfe/trunk/test/Driver/fast-math.c

Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -699,7 +699,8 @@
   Opts.Reciprocals = Args.getAllArgValues(OPT_mrecip_EQ);
   Opts.ReciprocalMath = Args.hasArg(OPT_freciprocal_math);
   Opts.NoTrappingMath = Args.hasArg(OPT_fno_trapping_math);
-  Opts.FPCastOverflowWorkaround = Args.hasArg(OPT_ffp_cast_overflow_workaround);
+  Opts.StrictFloatCastOverflow =
+  !Args.hasArg(OPT_fno_strict_float_cast_overflow);
 
   Opts.NoZeroInitializedInBSS = Args.hasArg(OPT_mno_zero_initialized_in_bss);
   Opts.NumRegisterParameters = getLastArgIntValue(Args, OPT_mregparm, 0, Diags);
Index: cfe/trunk/lib/CodeGen/CGCall.cpp
===
--- cfe/trunk/lib/CodeGen/CGCall.cpp
+++ cfe/trunk/lib/CodeGen/CGCall.cpp
@@ -1727,8 +1727,10 @@
 FuncAttrs.addAttribute("no-trapping-math",
llvm::toStringRef(CodeGenOpts.NoTrappingMath));
 
-if (CodeGenOpts.FPCastOverflowWorkaround)
-  FuncAttrs.addAttribute("fp-cast-overflow-workaround", "true");
+// Strict (compliant) code is the default, so only add this attribute to
+// indicate that we are trying to workaround a problem case.
+if (!CodeGenOpts.StrictFloatCastOverflow)
+  FuncAttrs.addAttribute("strict-float-cast-overflow", "false");
 
 // TODO: Are these all needed?
 // unsafe/inf/nan/nsz are handled by instruction-level FastMathFlags.
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -2243,9 +2243,9 @@
   }
 
   // Disable a codegen optimization for floating-point casts.
-  if (Args.hasFlag(options::OPT_ffp_cast_overflow_workaround,
-   options::OPT_fno_fp_cast_overflow_workaround, false))
-CmdArgs.push_back("-ffp-cast-overflow-workaround");
+  if (Args.hasFlag(options::OPT_fno_strict_float_cast_overflow,
+   options::OPT_fstrict_float_cast_overflow, false))
+CmdArgs.push_back("-fno-strict-float-cast-overflow");
 }
 
 static void RenderAnalyzerOptions(const ArgList , ArgStringList ,
Index: cfe/trunk/docs/UsersManual.rst
===
--- cfe/trunk/docs/UsersManual.rst
+++ cfe/trunk/docs/UsersManual.rst
@@ -1255,15 +1255,13 @@
flushed-to-zero number is preserved in the sign of 0, denormals are
flushed to positive zero, respectively.
 
-.. option:: -f[no-]fp-cast-overflow-workaround
+.. option:: -f[no-]strict-float-cast-overflow
 
-   Enable a workaround for code that casts floating-point values to 
-   integers and back to floating-point. If the floating-point value 
-   is not representable in the intermediate integer type, the code is
-   incorrect according to the language standard. This flag will attempt 
-   to generate code as if the result of an overflowing conversion matches
-   the overflowing behavior of a target's native float-to-int conversion
-   instructions.
+   When a floating-point value is not representable in a destination integer 
+   type, the code has undefined behavior according to the language standard.
+   By default, Clang will not guarantee any particular result in that case.
+   With the 'no-strict' option, Clang attempts to match the overflowing behavior
+   of the target's native float-to-int conversion instructions.
 
 .. option:: -fwhole-program-vtables
 
Index: cfe/trunk/docs/ReleaseNotes.rst
===
--- cfe/trunk/docs/ReleaseNotes.rst
+++ cfe/trunk/docs/ReleaseNotes.rst
@@ -89,14 +89,13 @@
 New Compiler Flags
 --
 
-- :option:`-ffp-cast-overflow-workaround` and
-  :option:`-fno-fp-cast-overflow-workaround`
-  enable (disable) a workaround for code that casts floating-point values to
-  integers and back to floating-point. If the floating-point value is not
-  representable in the intermediate integer type, the code is incorrect
-  according to the language standard. This 

[PATCH] D46236: [Driver, CodeGen] rename options to disable an FP cast optimization

[Sanjay Patel via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC331209: [Driver, CodeGen] rename options to disable an FP 
cast optimization (authored by spatel, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D46236

Files:
  docs/ReleaseNotes.rst
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGCall.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/no-junk-ftrunc.c
  test/Driver/fast-math.c

Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -137,10 +137,10 @@
 CODEGENOPT(FlushDenorm   , 1, 0) ///< Allow FP denorm numbers to be flushed to zero
 CODEGENOPT(CorrectlyRoundedDivSqrt, 1, 0) ///< -cl-fp32-correctly-rounded-divide-sqrt
 
-/// Disable a float-to-int-to-float cast optimization. This attempts to generate
-/// code as if the result of an overflowing conversion matches the overflowing
-/// behavior of a target's native float-to-int conversion instructions.
-CODEGENOPT(FPCastOverflowWorkaround, 1, 0)
+/// When false, this attempts to generate code as if the result of an
+/// overflowing conversion matches the overflowing behavior of a target's native
+/// float-to-int conversion instructions.
+CODEGENOPT(StrictFloatCastOverflow, 1, 1)
 
 CODEGENOPT(UniformWGSize , 1, 0) ///< -cl-uniform-work-group-size
 CODEGENOPT(NoZeroInitializedInBSS , 1, 0) ///< -fno-zero-initialized-in-bss.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1029,10 +1029,12 @@
   Flags<[CC1Option]>, HelpText<"Form fused FP ops (e.g. FMAs): fast (everywhere)"
   " | on (according to FP_CONTRACT pragma, default) | off (never fuse)">, Values<"fast,on,off">;
 
-def ffp_cast_overflow_workaround : Flag<["-"],
-  "ffp-cast-overflow-workaround">, Group, Flags<[CC1Option]>;
-def fno_fp_cast_overflow_workaround : Flag<["-"],
-  "fno-fp-cast-overflow-workaround">, Group, Flags<[CC1Option]>;
+def fstrict_float_cast_overflow : Flag<["-"],
+  "fstrict-float-cast-overflow">, Group, Flags<[CC1Option]>,
+  HelpText<"Assume that overflowing float-to-int casts are undefined (default)">;
+def fno_strict_float_cast_overflow : Flag<["-"],
+  "fno-strict-float-cast-overflow">, Group, Flags<[CC1Option]>,
+  HelpText<"Relax language rules and try to match the behavior of the target's native float-to-int conversion instructions">;
 
 def ffor_scope : Flag<["-"], "ffor-scope">, Group;
 def fno_for_scope : Flag<["-"], "fno-for-scope">, Group;
Index: test/Driver/fast-math.c
===
--- test/Driver/fast-math.c
+++ test/Driver/fast-math.c
@@ -289,25 +289,25 @@
 // CHECK-NO-TRAPPING-MATH: "-fno-trapping-math"
 
 // This isn't fast-math, but the option is handled in the same place as other FP params.
-// Last option wins, and the flag is *not* passed by default. 
+// Last option wins, and strict behavior is assumed by default. 
 
-// RUN: %clang -### -ffp-cast-overflow-workaround -c %s 2>&1 \
+// RUN: %clang -### -fno-strict-float-cast-overflow -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPOV-WORKAROUND %s
 // CHECK-FPOV-WORKAROUND: "-cc1"
-// CHECK-FPOV-WORKAROUND: "-ffp-cast-overflow-workaround"
+// CHECK-FPOV-WORKAROUND: "-fno-strict-float-cast-overflow"
 
 // RUN: %clang -### -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPOV-WORKAROUND-DEFAULT %s
 // CHECK-FPOV-WORKAROUND-DEFAULT: "-cc1"
-// CHECK-FPOV-WORKAROUND-DEFAULT-NOT: "-ffp-cast-overflow-workaround"
+// CHECK-FPOV-WORKAROUND-DEFAULT-NOT: "strict-float-cast-overflow"
 
-// RUN: %clang -### -fno-fp-cast-overflow-workaround -c %s 2>&1 \
+// RUN: %clang -### -fstrict-float-cast-overflow -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FPOV-WORKAROUND %s
 // CHECK-NO-FPOV-WORKAROUND: "-cc1"
-// CHECK-NO-FPOV-WORKAROUND-NOT: "-ffp-cast-overflow-workaround"
+// CHECK-NO-FPOV-WORKAROUND-NOT: "strict-float-cast-overflow"
 
-// RUN: %clang -### -ffp-cast-overflow-workaround -fno-fp-cast-overflow-workaround -c %s 2>&1 \
+// RUN: %clang -### -fno-strict-float-cast-overflow -fstrict-float-cast-overflow -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FPOV-WORKAROUND-OVERRIDE %s
 // CHECK-NO-FPOV-WORKAROUND-OVERRIDE: "-cc1"
-// CHECK-NO-FPOV-WORKAROUND-OVERRIDE-NOT: "-ffp-cast-overflow-workaround"
+// CHECK-NO-FPOV-WORKAROUND-OVERRIDE-NOT: "strict-float-cast-overflow"
 
Index: test/CodeGen/no-junk-ftrunc.c
===
--- test/CodeGen/no-junk-ftrunc.c
+++ test/CodeGen/no-junk-ftrunc.c
@@ -1,12 +1,16 @@
-// RUN: %clang_cc1 -S -ffp-cast-overflow-workaround %s -emit-llvm -o - | FileCheck %s
-// CHECK-LABEL: 

r331209 - [Driver, CodeGen] rename options to disable an FP cast optimization

[Sanjay Patel via cfe-commits]

Author: spatel
Date: Mon Apr 30 11:19:03 2018
New Revision: 331209

URL: http://llvm.org/viewvc/llvm-project?rev=331209=rev
Log:
[Driver, CodeGen] rename options to disable an FP cast optimization

As suggested in the post-commit thread for rL331056, we should match these 
clang options with the established vocabulary of the corresponding sanitizer
option. Also, the use of 'strict' is well-known for these kinds of knobs, 
and we can improve the descriptive text in the docs.

So this intends to match the logic of D46135 but only change the words.
Matching LLVM commit to match this spelling of the attribute to follow shortly.

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

Modified:
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/docs/UsersManual.rst
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGen/no-junk-ftrunc.c
cfe/trunk/test/Driver/fast-math.c

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=331209=331208=331209=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Mon Apr 30 11:19:03 2018
@@ -89,14 +89,13 @@ Non-comprehensive list of changes in thi
 New Compiler Flags
 --
 
-- :option:`-ffp-cast-overflow-workaround` and
-  :option:`-fno-fp-cast-overflow-workaround`
-  enable (disable) a workaround for code that casts floating-point values to
-  integers and back to floating-point. If the floating-point value is not
-  representable in the intermediate integer type, the code is incorrect
-  according to the language standard. This flag will attempt to generate code
-  as if the result of an overflowing conversion matches the overflowing 
behavior
-  of a target's native float-to-int conversion instructions.
+- :option:`-fstrict-float-cast-overflow` and
+  :option:`-fno-strict-float-cast-overflow` -
+   When a floating-point value is not representable in a destination integer
+   type, the code has undefined behavior according to the language standard.
+   By default, Clang will not guarantee any particular result in that case.
+   With the 'no-strict' option, Clang attempts to match the overflowing 
behavior
+   of the target's native float-to-int conversion instructions.
 
 - ...
 

Modified: cfe/trunk/docs/UsersManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=331209=331208=331209=diff
==
--- cfe/trunk/docs/UsersManual.rst (original)
+++ cfe/trunk/docs/UsersManual.rst Mon Apr 30 11:19:03 2018
@@ -1255,15 +1255,13 @@ are listed below.
flushed-to-zero number is preserved in the sign of 0, denormals are
flushed to positive zero, respectively.
 
-.. option:: -f[no-]fp-cast-overflow-workaround
+.. option:: -f[no-]strict-float-cast-overflow
 
-   Enable a workaround for code that casts floating-point values to 
-   integers and back to floating-point. If the floating-point value 
-   is not representable in the intermediate integer type, the code is
-   incorrect according to the language standard. This flag will attempt 
-   to generate code as if the result of an overflowing conversion matches
-   the overflowing behavior of a target's native float-to-int conversion
-   instructions.
+   When a floating-point value is not representable in a destination integer 
+   type, the code has undefined behavior according to the language standard.
+   By default, Clang will not guarantee any particular result in that case.
+   With the 'no-strict' option, Clang attempts to match the overflowing 
behavior
+   of the target's native float-to-int conversion instructions.
 
 .. option:: -fwhole-program-vtables
 

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=331209=331208=331209=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Apr 30 11:19:03 2018
@@ -1029,10 +1029,12 @@ def ffp_contract : Joined<["-"], "ffp-co
   Flags<[CC1Option]>, HelpText<"Form fused FP ops (e.g. FMAs): fast 
(everywhere)"
   " | on (according to FP_CONTRACT pragma, default) | off (never fuse)">, 
Values<"fast,on,off">;
 
-def ffp_cast_overflow_workaround : Flag<["-"],
-  "ffp-cast-overflow-workaround">, Group, Flags<[CC1Option]>;
-def fno_fp_cast_overflow_workaround : Flag<["-"],
-  "fno-fp-cast-overflow-workaround">, Group, Flags<[CC1Option]>;
+def fstrict_float_cast_overflow : Flag<["-"],
+  "fstrict-float-cast-overflow">, Group, Flags<[CC1Option]>,
+  HelpText<"Assume 

[PATCH] D46233: [ASTMatchers] Overload isConstexpr for ifStmts

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: docs/LibASTMatchersReference.html:179
 
-Matcherhttp://clang.llvm.org/doxygen/classclang_1_1Decl.html;>DeclcxxMethodDeclMatcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXMethodDecl.html;>CXXMethodDecl...
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1Decl.html;>DeclcxxMethodDeclMatcherCXXMethodDecl...
 Matches method 
declarations.

alexfh wrote:
> The changes don't look nice. It looks like either dump_ast_matchers.py can't 
> access network on your machine. Can this be the case? The URLs (e.g. 
> http://clang.llvm.org/doxygen/classclang_1_1CXXMethodDecl.html) seem to work 
> for me.
Just checked that the script still works fine.


Repository:
  rC Clang

https://reviews.llvm.org/D46233



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

r331207 - Regenerated AST Matchers doc.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Mon Apr 30 11:12:15 2018
New Revision: 331207

URL: http://llvm.org/viewvc/llvm-project?rev=331207=rev
Log:
Regenerated AST Matchers doc.

Backported a minor fix to the comment in the header.

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=331207=331206=331207=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Mon Apr 30 11:12:15 2018
@@ -1926,15 +1926,16 @@ Example matches a || b (matcher = binary
 
 
 
-Matcherhttp://clang.llvm.org/doxygen/classclang_1_1BinaryOperator.html;>BinaryOperatorisAssignmentOperator
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1BinaryOperator.html;>BinaryOperatorisAssignmentOperator
 Matches all 
kinds of assignment operators.
 
 Example 1: matches a += b (matcher = binaryOperator(isAssignmentOperator()))
   if (a == b)
 a += b;
 
-Example 2: matches s1 = s2 (matcher = 
cxxOperatorCallExpr(isAssignmentOperator()))
-  struct S { S& operator=(const S&); };
+Example 2: matches s1 = s2
+   (matcher = cxxOperatorCallExpr(isAssignmentOperator()))
+  struct S { S operator=(const S); };
   void x() { S s1, s2; s1 = s2; })
 
 
@@ -2319,15 +2320,16 @@ Usable as: Matcherhttp://cl
 
 
 
-Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXOperatorCallExpr.html;>CXXOperatorCallExprisAssignmentOperator
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXOperatorCallExpr.html;>CXXOperatorCallExprisAssignmentOperator
 Matches all 
kinds of assignment operators.
 
 Example 1: matches a += b (matcher = binaryOperator(isAssignmentOperator()))
   if (a == b)
 a += b;
 
-Example 2: matches s1 = s2 (matcher = 
cxxOperatorCallExpr(isAssignmentOperator()))
-  struct S { S& operator=(const S&); };
+Example 2: matches s1 = s2
+   (matcher = cxxOperatorCallExpr(isAssignmentOperator()))
+  struct S { S operator=(const S); };
   void x() { S s1, s2; s1 = s2; })
 
 

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=331207=331206=331207=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Mon Apr 30 11:12:15 2018
@@ -4038,7 +4038,7 @@ AST_POLYMORPHIC_MATCHER_P(hasOperatorNam
   return Name == Node.getOpcodeStr(Node.getOpcode());
 }
 
-/// \brief Matches on all kinds of assignment operators.
+/// \brief Matches all kinds of assignment operators.
 ///
 /// Example 1: matches a += b (matcher = 
binaryOperator(isAssignmentOperator()))
 /// \code


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

r331206 - [OPENMP] Do not crash on codegen for CXX member functions.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Apr 30 11:09:40 2018
New Revision: 331206

URL: http://llvm.org/viewvc/llvm-project?rev=331206=rev
Log:
[OPENMP] Do not crash on codegen for CXX member functions.

Non-static member functions should not be emitted as a standalone
functions, this leads to compiler crash.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/declare_target_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=331206=331205=331206=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Mon Apr 30 11:09:40 2018
@@ -898,6 +898,9 @@ static void EmitOMPAggregateInit(CodeGen
 
 static llvm::Optional
 isDeclareTargetDeclaration(const ValueDecl *VD) {
+  if (const auto *MD = dyn_cast(VD))
+if (!MD->isStatic())
+  return llvm::None;
   for (const Decl *D : VD->redecls()) {
 if (!D->hasAttrs())
   continue;

Modified: cfe/trunk/test/OpenMP/declare_target_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_target_codegen.cpp?rev=331206=331205=331206=diff
==
--- cfe/trunk/test/OpenMP/declare_target_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_target_codegen.cpp Mon Apr 30 11:09:40 2018
@@ -32,6 +32,11 @@ int baz2();
 int baz4() { return 5; }
 
 #pragma omp declare target
+struct S {
+  int a;
+  S(int a) : a(a) {}
+};
+
 int foo() { return 0; }
 int b = 15;
 int d;
@@ -47,6 +52,7 @@ int maini1() {
 #pragma omp target map(tofrom \
: a, b)
   {
+S s(a);
 static long aaa = 23;
 a = foo() + bar() + b + c + d + aa + aaa;
   }


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

[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

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

arthur.j.odwyer added a comment.

Sorry, I responded via email but I guess Phabricator didn't pick it up for
some reason.
See below.


Repository:
  rC Clang

https://reviews.llvm.org/D43322



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

[PATCH] D46233: [ASTMatchers] Overload isConstexpr for ifStmts

[Alexander Kornienko via Phabricator via cfe-commits]

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



Comment at: docs/LibASTMatchersReference.html:179
 
-Matcherhttp://clang.llvm.org/doxygen/classclang_1_1Decl.html;>DeclcxxMethodDeclMatcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXMethodDecl.html;>CXXMethodDecl...
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1Decl.html;>DeclcxxMethodDeclMatcherCXXMethodDecl...
 Matches method 
declarations.

The changes don't look nice. It looks like either dump_ast_matchers.py can't 
access network on your machine. Can this be the case? The URLs (e.g. 
http://clang.llvm.org/doxygen/classclang_1_1CXXMethodDecl.html) seem to work 
for me.


Repository:
  rC Clang

https://reviews.llvm.org/D46233



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

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added a comment.

> I think, it's 13, if you choose to remove stars, and 17 otherwise. The 
> difference is excessive spaces vs. required ones. Implementing proper logic 
> may be involved, but we can simplify it to something like "count all 
> non-space characters and a single space between words, but don't count spaces 
> around punctuation":

Isn't it a business of clang-format to determine the number of spaces between 
lexemes? IMO the solutuion you've provided for `GetTypeNameLength` is enough. 
Considering punctuation here is overkill :-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45927



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

[PATCH] D44788: Add an option to support debug fission on implicit ThinLTO.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:423
+Args.MakeArgString(Twine("-plugin-opt=objcopy=") + Objcopy));
+StringRef DWO_Dir = A->getValue();
+CmdArgs.push_back(

Can we default this to a path alongside the output file as @probinson suggested?


https://reviews.llvm.org/D44788



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

[PATCH] D45900: CodeGen: Fix invalid bitcast for lifetime.start/end

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In https://reviews.llvm.org/D45900#1074400, @rjmccall wrote:

> These functions must predate the addition of CreateLifetimeStart and 
> CreateLifetimeEnd methods on IRBuilder; we should just be calling those.


Sorry. I think I misunderstood your comments. I will revise again.


https://reviews.llvm.org/D45900



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

[PATCH] D46022: [OpenCL] Restrict various keywords in OpenCL C++ mode

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added inline comments.



Comment at: lib/Parse/ParseStmtAsm.cpp:696
+return StmtError();
+  }
+

rjmccall wrote:
> svenvh wrote:
> > rjmccall wrote:
> > > svenvh wrote:
> > > > rjmccall wrote:
> > > > > You might consider parsing the statement normally and then just 
> > > > > diagnosing after the fact, maybe in Sema.  You'd have to add the 
> > > > > check in a couple different places, though.
> > > > Precisely the reason why I did it in the parser, otherwise we'd have to 
> > > > duplicate the check in multiple places.
> > > The downside of this is that the parser recovery is probably very bad.  
> > > But since this is asm, it's not likely to matter too much.
> > > 
> > > ...is this *also* really only an OpenCL C++ restriction?  Maybe we should 
> > > read this one as a general restriction that happens to have been 
> > > overlooked in the OpenCL C spec.
> > Yes, `asm` is only explicitly restricted in OpenCL C++.  Not sure if it's 
> > safe to assume `asm` has been overlooked for OpenCL C though, it's not 
> > explicitly forbidden so people may be using it.
> Do you have any contact with the OpenCL committee?  You might want to collect 
> these issues and present them to them.  They may just not be aware that `asm` 
> is just as much a feature in C as it is in C++, and it would be good to 
> understand what the purpose of the `goto` restriction is.
Yes, we can clarify this with Khronos.  Shall I leave the `asm` and `goto` 
restrictions out of this patch for now until we have clarification?


https://reviews.llvm.org/D46022



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

[PATCH] D46022: [OpenCL] Restrict various keywords in OpenCL C++ mode

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh updated this revision to Diff 144583.
svenvh added a comment.

Moved thread storage class specifier diagnosing to `ActOnVariableDeclarator`.


https://reviews.llvm.org/D46022

Files:
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/TokenKinds.def
  lib/Basic/IdentifierTable.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseStmtAsm.cpp
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaStmt.cpp
  test/Parser/opencl-cl20.cl
  test/Parser/opencl-cxx-keywords.cl
  test/Parser/opencl-storage-class.cl
  test/SemaOpenCL/storageclass.cl
  test/SemaOpenCLCXX/restricted.cl

Index: test/SemaOpenCLCXX/restricted.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/restricted.cl
@@ -0,0 +1,77 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify -fsyntax-only
+
+// This test checks that various C/C++/OpenCL C constructs are not available in
+// OpenCL C++, according to OpenCL C++ 1.0 Specification Section 2.9.
+
+// Test that typeid is not available in OpenCL C++.
+namespace std {
+  // Provide a dummy std::type_info so that we can use typeid.
+  class type_info {
+int a;
+  };
+}
+__constant std::type_info int_ti = typeid(int);
+// expected-error@-1 {{'typeid' is not supported in OpenCL C++}}
+
+// Test that dynamic_cast is not available in OpenCL C++.
+class A {
+public:
+  int a;
+};
+
+class B : public A {
+  int b;
+};
+
+B *test_dynamic_cast(B *p) {
+  return dynamic_cast(p);
+  // expected-error@-1 {{'dynamic_cast' is not supported in OpenCL C++}}
+}
+
+// Test storage class qualifiers.
+__constant _Thread_local int a = 1;
+// expected-error@-1 {{OpenCL C++ version 1.0 does not support the '_Thread_local' storage class specifier}}
+__constant __thread int b = 2;
+// expected-error@-1 {{OpenCL C++ version 1.0 does not support the '__thread' storage class specifier}}
+kernel void test_storage_classes() {
+  register int x;
+  // expected-error@-1 {{OpenCL C++ version 1.0 does not support the 'register' storage class specifier}}
+  thread_local int y;
+  // expected-error@-1 {{OpenCL C++ version 1.0 does not support the 'thread_local' storage class specifier}}
+}
+
+// Test that access qualifiers are reserved keywords.
+kernel void test_access_qualifiers() {
+  int read_only;
+  // expected-error@-1 {{'read_only' is a reserved keyword in OpenCL C++}}
+  // expected-warning@-2 {{declaration does not declare anything}}
+  int __read_only;
+  // expected-error@-1 {{'__read_only' is a reserved keyword in OpenCL C++}}
+  // expected-warning@-2 {{declaration does not declare anything}}
+  int write_only;
+  // expected-error@-1 {{'write_only' is a reserved keyword in OpenCL C++}}
+  // expected-warning@-2 {{declaration does not declare anything}}
+  int __write_only;
+  // expected-error@-1 {{'__write_only' is a reserved keyword in OpenCL C++}}
+  // expected-warning@-2 {{declaration does not declare anything}}
+  int read_write;
+  // expected-error@-1 {{'read_write' is a reserved keyword in OpenCL C++}}
+  // expected-warning@-2 {{declaration does not declare anything}}
+  int __read_write;
+  // expected-error@-1 {{'__read_write' is a reserved keyword in OpenCL C++}}
+  // expected-warning@-2 {{declaration does not declare anything}}
+}
+
+// Test other keywords that are not available in OpenCL C++.
+kernel void test_misc() {
+  goto label;
+  // expected-error@-1 {{'goto' is not supported in OpenCL C++}}
+  goto *&
+  // expected-error@-1 {{'goto' is not supported in OpenCL C++}}
+  // expected-warning@-2 {{use of GNU indirect-goto extension}}
+  // expected-warning@-3 {{use of GNU address-of-label extension}}
+label:
+  asm("");
+  // expected-error@-1 {{'asm' is not supported in OpenCL C++}}
+  // expected-warning@-2 {{expression result unused}}
+}
Index: test/SemaOpenCL/storageclass.cl
===
--- test/SemaOpenCL/storageclass.cl
+++ test/SemaOpenCL/storageclass.cl
@@ -10,14 +10,14 @@
 static global float g_global_static_var = 0;   // expected-error {{program scope variable must reside in constant address space}}
 static local float g_local_static_var = 0; // expected-error {{program scope variable must reside in constant address space}}
 static private float g_private_static_var = 0; // expected-error {{program scope variable must reside in constant address space}}
-static generic float g_generic_static_var = 0; // expected-error{{OpenCL version 1.2 does not support the 'generic' type qualifier}} // expected-error {{program scope variable must reside in constant address space}}
+static generic float g_generic_static_var = 0; // expected-error{{OpenCL C version 1.2 does not support the 'generic' type qualifier}} // expected-error {{program scope variable must reside in 

[PATCH] D44788: Add an option to support debug fission on implicit ThinLTO.

[Yunlian Jiang via Phabricator via cfe-commits]

yunlian added a comment.

ping?


https://reviews.llvm.org/D44788



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

[PATCH] D45900: CodeGen: Fix invalid bitcast for lifetime.start/end

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 144574.
yaxunl added a comment.

Revised by John's comments.


https://reviews.llvm.org/D45900

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGenCXX/amdgcn_declspec_get.cpp

Index: test/CodeGenCXX/amdgcn_declspec_get.cpp
===
--- /dev/null
+++ test/CodeGenCXX/amdgcn_declspec_get.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm -O3 -fdeclspec \
+// RUN: -disable-llvm-passes -o - %s | FileCheck %s
+
+int get_x();
+
+struct A {
+   __declspec(property(get = _get_x)) int x;
+   static int _get_x(void) {
+ return get_x();
+   };
+};
+
+extern const A a;
+
+// CHECK-LABEL: define void @_Z4testv()
+// CHECK:  %i = alloca i32, align 4, addrspace(5)
+// CHECK:  %[[ii:.*]] = addrspacecast i32 addrspace(5)* %i to i32*
+// CHECK:  %[[cast1:.*]] = addrspacecast i32* %[[ii]] to i8 addrspace(5)*
+// CHECK:  call void @llvm.lifetime.start.p5i8(i64 4, i8 addrspace(5)* %[[cast1]])
+// CHECK:  %call = call i32 @_ZN1A6_get_xEv()
+// CHECK:  store i32 %call, i32* %[[ii]]
+// CHECK:  %[[cast2:.*]] = addrspacecast i32* %[[ii]] to i8 addrspace(5)*
+// CHECK:  call void @llvm.lifetime.end.p5i8(i64 4, i8 addrspace(5)* %[[cast2]])
+void test()
+{
+  int i = a.x;
+}
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -418,7 +418,7 @@
 : Addr(addr.getPointer()), Size(size) {}
 
 void Emit(CodeGenFunction , Flags flags) override {
-  CGF.EmitLifetimeEnd(Size, Addr);
+  CGF.EmitLifetimeEnd(Size, CGF.EmitCastToVoidPtrInAllocaAddrSpace(Addr));
 }
   };
 
@@ -2084,6 +2084,9 @@
   /// Emit a cast to void* in the appropriate address space.
   llvm::Value *EmitCastToVoidPtr(llvm::Value *value);
 
+  /// Emit a cast to void* in alloca address space.
+  llvm::Value *EmitCastToVoidPtrInAllocaAddrSpace(llvm::Value *V);
+
   /// EvaluateExprAsBool - Perform the usual unary conversions on the specified
   /// expression and compare the result against zero, returning an Int1Ty value.
   llvm::Value *EvaluateExprAsBool(const Expr *E);
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -59,6 +59,18 @@
   return Builder.CreateBitCast(value, destType);
 }
 
+llvm::Value *
+CodeGenFunction::EmitCastToVoidPtrInAllocaAddrSpace(llvm::Value *V) {
+  if (V->getType()->getPointerAddressSpace() !=
+  CGM.getDataLayout().getAllocaAddrSpace()) {
+return getTargetHooks().performAddrSpaceCast(
+*this, V, getASTAllocaAddressSpace(), LangAS::Default, AllocaInt8PtrTy,
+/*non-null*/ true);
+  } else {
+return Builder.CreateBitCast(V, AllocaInt8PtrTy);
+  }
+}
+
 /// CreateTempAlloca - This creates a alloca and inserts it into the entry
 /// block.
 Address CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, CharUnits Align,
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -965,6 +965,9 @@
   if (!ShouldEmitLifetimeMarkers)
 return nullptr;
 
+  assert(Addr->getType()->getPointerAddressSpace() ==
+ CGM.getDataLayout().getAllocaAddrSpace() &&
+ "Pointer argument must be in alloca address space");
   llvm::Value *SizeV = llvm::ConstantInt::get(Int64Ty, Size);
   Addr = Builder.CreateBitCast(Addr, AllocaInt8PtrTy);
   llvm::CallInst *C =
@@ -974,6 +977,9 @@
 }
 
 void CodeGenFunction::EmitLifetimeEnd(llvm::Value *Size, llvm::Value *Addr) {
+  assert(Addr->getType()->getPointerAddressSpace() ==
+ CGM.getDataLayout().getAllocaAddrSpace() &&
+ "Pointer argument must be in alloca address space");
   Addr = Builder.CreateBitCast(Addr, AllocaInt8PtrTy);
   llvm::CallInst *C =
   Builder.CreateCall(CGM.getLLVMLifetimeEndFn(), {Size, Addr});
@@ -1175,8 +1181,10 @@
 if (!Bypasses.IsBypassed() &&
 !(!getLangOpts().CPlusPlus && hasLabelBeenSeenInCurrentScope())) {
   uint64_t size = CGM.getDataLayout().getTypeAllocSize(allocaTy);
-  emission.SizeForLifetimeMarkers =
-  EmitLifetimeStart(size, address.getPointer());
+  auto *VoidPtr = address.getPointer();
+  if (ShouldEmitLifetimeMarkers)
+VoidPtr = EmitCastToVoidPtrInAllocaAddrSpace(VoidPtr);
+  emission.SizeForLifetimeMarkers = EmitLifetimeStart(size, VoidPtr);
 }
   } else {
 assert(!emission.useLifetimeMarkers());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r331195 - [OPENMP] Do not crash on incorrect input data.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Apr 30 09:26:57 2018
New Revision: 331195

URL: http://llvm.org/viewvc/llvm-project?rev=331195=rev
Log:
[OPENMP] Do not crash on incorrect input data.

Emit error messages instead of compiler crashing when the target region
does not exist in the device code + fix crash when the location comes
from macros.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/target_codegen.cpp
cfe/trunk/test/OpenMP/target_messages.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=331195=331194=331195=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Mon Apr 30 09:26:57 2018
@@ -2588,21 +2588,20 @@ llvm::Function *CGOpenMPRuntime::emitThr
 static void getTargetEntryUniqueInfo(ASTContext , SourceLocation Loc,
  unsigned , unsigned ,
  unsigned ) {
-
   SourceManager  = C.getSourceManager();
 
   // The loc should be always valid and have a file ID (the user cannot use
   // #pragma directives in macros)
 
   assert(Loc.isValid() && "Source location is expected to be always valid.");
-  assert(Loc.isFileID() && "Source location is expected to refer to a file.");
 
   PresumedLoc PLoc = SM.getPresumedLoc(Loc);
   assert(PLoc.isValid() && "Source location is expected to be always valid.");
 
   llvm::sys::fs::UniqueID ID;
-  if (llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID))
-llvm_unreachable("Source file with target region no longer exists!");
+  if (auto EC = llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID))
+SM.getDiagnostics().Report(diag::err_cannot_open_file)
+<< PLoc.getFilename() << EC.message();
 
   DeviceID = ID.getDevice();
   FileID = ID.getFile();
@@ -3586,8 +3585,13 @@ void CGOpenMPRuntime::OffloadEntriesInfo
   // If we are emitting code for a target, the entry is already initialized,
   // only has to be registered.
   if (CGM.getLangOpts().OpenMPIsDevice) {
-assert(hasTargetRegionEntryInfo(DeviceID, FileID, ParentName, LineNum) &&
-   "Entry must exist.");
+if (!hasTargetRegionEntryInfo(DeviceID, FileID, ParentName, LineNum)) {
+  unsigned DiagID = CGM.getDiags().getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "Unable to find target region on line '%0' in the device code.");
+  CGM.getDiags().Report(DiagID) << LineNum;
+  return;
+}
 auto  =
 OffloadEntriesTargetRegion[DeviceID][FileID][ParentName][LineNum];
 assert(Entry.isValid() && "Entry not initialized!");
@@ -3928,14 +3932,27 @@ void CGOpenMPRuntime::createOffloadEntri
 if (const auto *CE =
 
dyn_cast(
 E)) {
-  assert(CE->getID() && CE->getAddress() &&
- "Entry ID and Addr are invalid!");
+  if (!CE->getID() || !CE->getAddress()) {
+unsigned DiagID = CGM.getDiags().getCustomDiagID(
+DiagnosticsEngine::Error,
+"Offloading entry for target region is incorect: either the "
+"address or the ID is invalid.");
+CGM.getDiags().Report(DiagID);
+continue;
+  }
   createOffloadEntry(CE->getID(), CE->getAddress(), /*Size=*/0,
  CE->getFlags(), llvm::GlobalValue::WeakAnyLinkage);
 } else if (const auto *CE =
dyn_cast(E)) {
-  assert(CE->getAddress() && "Entry Addr is invalid!");
+  if (!CE->getAddress()) {
+unsigned DiagID = CGM.getDiags().getCustomDiagID(
+DiagnosticsEngine::Error,
+"Offloading entry for declare target varible is inccorect: the "
+"address is invalid.");
+CGM.getDiags().Report(DiagID);
+continue;
+  }
   createOffloadEntry(CE->getAddress(), CE->getAddress(),
  CE->getVarSize().getQuantity(), CE->getFlags(),
  CE->getLinkage());
@@ -3958,15 +3975,23 @@ void CGOpenMPRuntime::loadOffloadInfoMet
 return;
 
   auto Buf = llvm::MemoryBuffer::getFile(CGM.getLangOpts().OMPHostIRFile);
-  if (Buf.getError())
+  if (auto EC = Buf.getError()) {
+CGM.getDiags().Report(diag::err_cannot_open_file)
+<< CGM.getLangOpts().OMPHostIRFile << EC.message();
 return;
+  }
 
   llvm::LLVMContext C;
   auto ME = expectedToErrorOrAndEmitErrors(
   C, llvm::parseBitcodeFile(Buf.get()->getMemBufferRef(), C));
 
-  if (ME.getError())
+  if (auto EC = ME.getError()) {
+unsigned DiagID = CGM.getDiags().getCustomDiagID(
+DiagnosticsEngine::Error, "Unable to parse host IR file '%0':'%1'");
+CGM.getDiags().Report(DiagID)
+<< CGM.getLangOpts().OMPHostIRFile << EC.message();
 return;
+  }
 
   llvm::NamedMDNode *MD = ME.get()->getNamedMetadata("omp_offload.info");
   if (!MD)

Modified: 

[PATCH] D46035: [clangd] Fix unicode handling, using UTF-16 where LSP requires it.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/trunk/clangd/SourceCode.cpp:38
+// 0xxx is ASCII, handled above. 10xxx is a trailing byte, invalid here.
+// 1xxx is not valid UTF-8 at all. Assert because it's probably our 
bug.
+assert((UTF8Length >= 2 && UTF8Length <= 4) &&

benhamilton wrote:
> sammccall wrote:
> > benhamilton wrote:
> > > sammccall wrote:
> > > > benhamilton wrote:
> > > > > This is user input, right? Have we actually checked for valid UTF-8, 
> > > > > or do we just assume it's valid?
> > > > > 
> > > > > If not, it seems like an assertion is not the right check, but we 
> > > > > should reject it when we're reading the input.
> > > > > 
> > > > Yeah, I wasn't sure about this, offline discussion tentatively 
> > > > concluded we wanted an assert, but I'm happy to switch to something 
> > > > else.
> > > > 
> > > > We don't validate the code on the way in, so strings are "bytes of 
> > > > presumed-UTF8". This is usually not a big pain actually. But we 
> > > > could/should certainly make the JSON parser validate the UTF-8. (If we 
> > > > want to go this route, D45753 should be resolved first).
> > > > 
> > > > There's two ways the assertion could fire: the code is invalid UTF-8, 
> > > > or there's a bug in the unicode logic here. I thought the latter was 
> > > > more likely at least in the short-term :) and this is the least 
> > > > invasive way to catch it. And if a developer build (assert-enabled) 
> > > > crashes because an editor feeds it invalid bytes, then that's probably 
> > > > better than doing nothing (though not as good as catching the error 
> > > > earlier).
> > > The problem with not validating is it's easy to cause OOB memory access 
> > > (and thus security issues) if someone crafts malicious UTF-8 and makes us 
> > > read off the end of a string.
> > > 
> > > We should be clear about the status of all strings in the documentation 
> > > to APIs.
> > You still have to find/write a UTF-8 decoder that doesn't check bounds, 
> > which is (hopefully!) the harder part of writing that bug :-)
> > But I agree in principle, there's more subtle attacks too, like `C08A` 
> > which is invalid but non-validating decoders will treat as a newline.
> > 
> > I have a nearly-finished patch to add real validation to the JSON library, 
> > I'll copy you on it.
> Seems like this particular decoder isn't checking bounds, eh? ;)
> 
> If `NDEBUG` is set, it will happily set `UTF8Length` to however many leading 
> 1s there are (valid or not) and pass that to `CB(UTF8Length)`. It's true that 
> the current callbacks passed in won't directly turn that into an OOB memory 
> access, but they will end up returning an invalid UTF-16 code unit length 
> from `positionToOffset()`, so who knows what that will end up doing.
> 
> Thanks, I'm always happy to review Unicode stuff.
Ah, yeah - the implicit context here is that we don't do anything with UTF-16 
other than send it back to the client. So this is garbage in, garbage out. 
Definitely not ideal.


Repository:
  rL LLVM

https://reviews.llvm.org/D46035



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

[PATCH] D46035: [clangd] Fix unicode handling, using UTF-16 where LSP requires it.

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: clang-tools-extra/trunk/clangd/SourceCode.cpp:38
+// 0xxx is ASCII, handled above. 10xxx is a trailing byte, invalid here.
+// 1xxx is not valid UTF-8 at all. Assert because it's probably our 
bug.
+assert((UTF8Length >= 2 && UTF8Length <= 4) &&

sammccall wrote:
> benhamilton wrote:
> > sammccall wrote:
> > > benhamilton wrote:
> > > > This is user input, right? Have we actually checked for valid UTF-8, or 
> > > > do we just assume it's valid?
> > > > 
> > > > If not, it seems like an assertion is not the right check, but we 
> > > > should reject it when we're reading the input.
> > > > 
> > > Yeah, I wasn't sure about this, offline discussion tentatively concluded 
> > > we wanted an assert, but I'm happy to switch to something else.
> > > 
> > > We don't validate the code on the way in, so strings are "bytes of 
> > > presumed-UTF8". This is usually not a big pain actually. But we 
> > > could/should certainly make the JSON parser validate the UTF-8. (If we 
> > > want to go this route, D45753 should be resolved first).
> > > 
> > > There's two ways the assertion could fire: the code is invalid UTF-8, or 
> > > there's a bug in the unicode logic here. I thought the latter was more 
> > > likely at least in the short-term :) and this is the least invasive way 
> > > to catch it. And if a developer build (assert-enabled) crashes because an 
> > > editor feeds it invalid bytes, then that's probably better than doing 
> > > nothing (though not as good as catching the error earlier).
> > The problem with not validating is it's easy to cause OOB memory access 
> > (and thus security issues) if someone crafts malicious UTF-8 and makes us 
> > read off the end of a string.
> > 
> > We should be clear about the status of all strings in the documentation to 
> > APIs.
> You still have to find/write a UTF-8 decoder that doesn't check bounds, which 
> is (hopefully!) the harder part of writing that bug :-)
> But I agree in principle, there's more subtle attacks too, like `C08A` which 
> is invalid but non-validating decoders will treat as a newline.
> 
> I have a nearly-finished patch to add real validation to the JSON library, 
> I'll copy you on it.
Seems like this particular decoder isn't checking bounds, eh? ;)

If `NDEBUG` is set, it will happily set `UTF8Length` to however many leading 1s 
there are (valid or not) and pass that to `CB(UTF8Length)`. It's true that the 
current callbacks passed in won't directly turn that into an OOB memory access, 
but they will end up returning an invalid UTF-16 code unit length from 
`positionToOffset()`, so who knows what that will end up doing.

Thanks, I'm always happy to review Unicode stuff.


Repository:
  rL LLVM

https://reviews.llvm.org/D46035



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

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Actually, just ignoring spaces may be not the best option.

In https://reviews.llvm.org/D45927#1074593, @zinovy.nis wrote:

> > I think spaces that will be removed should be counted - long long is 9.
>
> I thought about it, but what about "long   long  int
>
> - * * *"? Is it still 9?


I think, it's 13, if you choose to remove stars, and 17 otherwise. The 
difference is excessive spaces vs. required ones. Implementing proper logic may 
be involved, but we can simplify it to something like "count all non-space 
characters and a single space between words, but don't count spaces around 
punctuation":

  enum CharType { Space, Alpha, Punctuation };
  CharType LastChar = Space, BeforeSpace = Punctuation;
  int NumChars = 0;
  for (char C : S) {
CharType NextChar = std::isalnum(C) ? Alpha : (std::isspace(C) || 
RemoveStars && C == '*') ? Space : Punctuation;
if (NextChar != Space) {
  ++NumChars; // Count the non-space character.
  if (LastChar == Space && NextChar == Alpha && BeforeSpace == Alpha)
++NumChars; // Count a single space character between two words.
  BeforeSpace = NextChar;
}
LastChar = NextChar;
  }




Comment at: clang-tidy/modernize/UseAutoCheck.cpp:445-449
+size_t UseAutoCheck::GetTypeNameLength(const SourceRange ,
+   const ASTContext ) {
+  const StringRef S = tooling::fixit::getText(SR, Context);
+  return std::count_if(S.begin(), S.end(), SpacePredicate);
+}

zinovy.nis wrote:
> alexfh wrote:
> > ```
> > static size_t GetTypeNameLength(const TypeLoc , const ASTContext 
> > , bool IgnoreStars) {
> >   const StringRef S = tooling::fixit::getText(Loc.getSourceRange(), 
> > Context);
> >   if (IgnoreStars)
> > return llvm::count_if(S, [] (char C) { return std::isspace(C) || C == 
> > '*'; });
> >   return llvm::count_if(S, [] (char C) { return std::isspace(C); });
> > }
> > ```
> `IgnoreStars` is initialized once in the ctor and is used widely for all the 
> literals in the translation unit.
> IMHO it's better to eliminate branches on `IgnoreStars`.
> IMHO it's better to eliminate branches on IgnoreStars.

Why do you think so?

I think that spreading the implementation of a rather trivial function to one 
method, one data member, two functions and a constructor makes the code 
significantly harder to read. And you for some reason propose to avoid a single 
branch that doesn't seem to be on any hot path. Am I missing something?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45927



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

[PATCH] D46035: [clangd] Fix unicode handling, using UTF-16 where LSP requires it.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/trunk/clangd/SourceCode.cpp:38
+// 0xxx is ASCII, handled above. 10xxx is a trailing byte, invalid here.
+// 1xxx is not valid UTF-8 at all. Assert because it's probably our 
bug.
+assert((UTF8Length >= 2 && UTF8Length <= 4) &&

benhamilton wrote:
> sammccall wrote:
> > benhamilton wrote:
> > > This is user input, right? Have we actually checked for valid UTF-8, or 
> > > do we just assume it's valid?
> > > 
> > > If not, it seems like an assertion is not the right check, but we should 
> > > reject it when we're reading the input.
> > > 
> > Yeah, I wasn't sure about this, offline discussion tentatively concluded we 
> > wanted an assert, but I'm happy to switch to something else.
> > 
> > We don't validate the code on the way in, so strings are "bytes of 
> > presumed-UTF8". This is usually not a big pain actually. But we 
> > could/should certainly make the JSON parser validate the UTF-8. (If we want 
> > to go this route, D45753 should be resolved first).
> > 
> > There's two ways the assertion could fire: the code is invalid UTF-8, or 
> > there's a bug in the unicode logic here. I thought the latter was more 
> > likely at least in the short-term :) and this is the least invasive way to 
> > catch it. And if a developer build (assert-enabled) crashes because an 
> > editor feeds it invalid bytes, then that's probably better than doing 
> > nothing (though not as good as catching the error earlier).
> The problem with not validating is it's easy to cause OOB memory access (and 
> thus security issues) if someone crafts malicious UTF-8 and makes us read off 
> the end of a string.
> 
> We should be clear about the status of all strings in the documentation to 
> APIs.
You still have to find/write a UTF-8 decoder that doesn't check bounds, which 
is (hopefully!) the harder part of writing that bug :-)
But I agree in principle, there's more subtle attacks too, like `C08A` which is 
invalid but non-validating decoders will treat as a newline.

I have a nearly-finished patch to add real validation to the JSON library, I'll 
copy you on it.


Repository:
  rL LLVM

https://reviews.llvm.org/D46035



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

[PATCH] D46035: [clangd] Fix unicode handling, using UTF-16 where LSP requires it.

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: clang-tools-extra/trunk/clangd/SourceCode.cpp:38
+// 0xxx is ASCII, handled above. 10xxx is a trailing byte, invalid here.
+// 1xxx is not valid UTF-8 at all. Assert because it's probably our 
bug.
+assert((UTF8Length >= 2 && UTF8Length <= 4) &&

sammccall wrote:
> benhamilton wrote:
> > This is user input, right? Have we actually checked for valid UTF-8, or do 
> > we just assume it's valid?
> > 
> > If not, it seems like an assertion is not the right check, but we should 
> > reject it when we're reading the input.
> > 
> Yeah, I wasn't sure about this, offline discussion tentatively concluded we 
> wanted an assert, but I'm happy to switch to something else.
> 
> We don't validate the code on the way in, so strings are "bytes of 
> presumed-UTF8". This is usually not a big pain actually. But we could/should 
> certainly make the JSON parser validate the UTF-8. (If we want to go this 
> route, D45753 should be resolved first).
> 
> There's two ways the assertion could fire: the code is invalid UTF-8, or 
> there's a bug in the unicode logic here. I thought the latter was more likely 
> at least in the short-term :) and this is the least invasive way to catch it. 
> And if a developer build (assert-enabled) crashes because an editor feeds it 
> invalid bytes, then that's probably better than doing nothing (though not as 
> good as catching the error earlier).
The problem with not validating is it's easy to cause OOB memory access (and 
thus security issues) if someone crafts malicious UTF-8 and makes us read off 
the end of a string.

We should be clear about the status of all strings in the documentation to APIs.


Repository:
  rL LLVM

https://reviews.llvm.org/D46035



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

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added inline comments.



Comment at: clang-tidy/modernize/UseAutoCheck.cpp:33
+
+bool IsNotSpace(const char& C) {
+  return !std::isspace(static_cast(C));

alexfh wrote:
> Why `const char&` and not just `char`? Moreover, these two functions can be 
> replaced with lambdas. See below.
Agree.



Comment at: clang-tidy/modernize/UseAutoCheck.cpp:445-449
+size_t UseAutoCheck::GetTypeNameLength(const SourceRange ,
+   const ASTContext ) {
+  const StringRef S = tooling::fixit::getText(SR, Context);
+  return std::count_if(S.begin(), S.end(), SpacePredicate);
+}

alexfh wrote:
> ```
> static size_t GetTypeNameLength(const TypeLoc , const ASTContext 
> , bool IgnoreStars) {
>   const StringRef S = tooling::fixit::getText(Loc.getSourceRange(), Context);
>   if (IgnoreStars)
> return llvm::count_if(S, [] (char C) { return std::isspace(C) || C == 
> '*'; });
>   return llvm::count_if(S, [] (char C) { return std::isspace(C); });
> }
> ```
`IgnoreStars` is initialized once in the ctor and is used widely for all the 
literals in the translation unit.
IMHO it's better to eliminate branches on `IgnoreStars`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45927



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

[PATCH] D45717: [clangd] Using index for GoToDefinition.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE331189: [clangd] Using index for GoToDefinition. (authored 
by hokein, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45717?vs=144561=144566#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45717

Files:
  clangd/ClangdServer.cpp
  clangd/XRefs.cpp
  clangd/XRefs.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -13,11 +13,14 @@
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "XRefs.h"
+#include "gmock/gmock.h"
+#include "index/FileIndex.h"
+#include "index/SymbolCollector.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Frontend/Utils.h"
+#include "clang/Index/IndexingAction.h"
 #include "llvm/Support/Path.h"
-#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -37,17 +40,33 @@
 };
 
 // FIXME: this is duplicated with FileIndexTests. Share it.
-ParsedAST build(StringRef Code) {
-  auto CI = createInvocationFromCommandLine(
-  {"clang", "-xc++", testPath("Foo.cpp").c_str()});
-  auto Buf = MemoryBuffer::getMemBuffer(Code);
+ParsedAST build(StringRef MainCode, StringRef HeaderCode = "") {
+  auto HeaderPath = testPath("foo.h");
+  auto MainPath = testPath("foo.cpp");
+  llvm::IntrusiveRefCntPtr VFS(
+  new vfs::InMemoryFileSystem());
+  VFS->addFile(MainPath, 0, llvm::MemoryBuffer::getMemBuffer(MainCode));
+  VFS->addFile(HeaderPath, 0, llvm::MemoryBuffer::getMemBuffer(HeaderCode));
+  std::vector Cmd = {"clang", "-xc++", MainPath.c_str()};
+  if (!HeaderCode.empty()) {
+std::vector args = {"-include", HeaderPath.c_str()};
+Cmd.insert(Cmd.begin() + 1, args.begin(), args.end());
+  }
+  auto CI = createInvocationFromCommandLine(Cmd);
+
+  auto Buf = MemoryBuffer::getMemBuffer(MainCode);
   auto AST = ParsedAST::Build(std::move(CI), nullptr, std::move(Buf),
-  std::make_shared(),
-  vfs::getRealFileSystem());
+  std::make_shared(), VFS);
   assert(AST.hasValue());
   return std::move(*AST);
 }
 
+std::unique_ptr buildIndex(StringRef MainCode,
+StringRef HeaderCode) {
+  auto AST = build(MainCode, HeaderCode);
+  return MemIndex::build(indexAST());
+}
+
 // Extracts ranges from an annotated example, and constructs a matcher for a
 // highlight set. Ranges should be named $read/$write as appropriate.
 Matcher &>
@@ -106,6 +125,65 @@
 
 MATCHER_P(RangeIs, R, "") { return arg.range == R; }
 
+TEST(GoToDefinition, WithIndex) {
+  Annotations SymbolHeader(R"cpp(
+class $forward[[Forward]];
+class $foo[[Foo]] {};
+
+void $f1[[f1]]();
+
+inline void $f2[[f2]]() {}
+  )cpp");
+  Annotations SymbolCpp(R"cpp(
+  class $forward[[forward]] {};
+  void  $f1[[f1]]() {}
+)cpp");
+
+  auto Index = buildIndex(SymbolCpp.code(), SymbolHeader.code());
+  auto runFindDefinitionsWithIndex = [](const Annotations ) {
+auto AST = build(/*MainCode=*/Main.code(),
+ /*HeaderCode=*/"");
+return clangd::findDefinitions(AST, Main.point(), Index.get());
+  };
+
+  Annotations Test(R"cpp(// only declaration in AST.
+void [[f1]]();
+int main() {
+  ^f1();
+}
+  )cpp");
+  EXPECT_THAT(runFindDefinitionsWithIndex(Test),
+  testing::ElementsAreArray(
+  {RangeIs(SymbolCpp.range("f1")), RangeIs(Test.range())}));
+
+  Test = Annotations(R"cpp(// definition in AST.
+void [[f1]]() {}
+int main() {
+  ^f1();
+}
+  )cpp");
+  EXPECT_THAT(runFindDefinitionsWithIndex(Test),
+  testing::ElementsAreArray(
+  {RangeIs(Test.range()), RangeIs(SymbolHeader.range("f1"))}));
+
+  Test = Annotations(R"cpp(// forward declaration in AST.
+class [[Foo]];
+F^oo* create();
+  )cpp");
+  EXPECT_THAT(runFindDefinitionsWithIndex(Test),
+  testing::ElementsAreArray(
+  {RangeIs(SymbolHeader.range("foo")), RangeIs(Test.range())}));
+
+  Test = Annotations(R"cpp(// defintion in AST.
+class [[Forward]] {};
+F^orward create();
+  )cpp");
+  EXPECT_THAT(runFindDefinitionsWithIndex(Test),
+  testing::ElementsAreArray({
+  RangeIs(Test.range()), RangeIs(SymbolHeader.range("forward")),
+  }));
+}
+
 TEST(GoToDefinition, All) {
   const char *Tests[] = {
   R"cpp(// Local variable
Index: clangd/XRefs.h
===
--- clangd/XRefs.h
+++ clangd/XRefs.h
@@ -15,13 +15,15 @@
 
 #include "ClangdUnit.h"
 #include "Protocol.h"

[clang-tools-extra] r331189 - [clangd] Using index for GoToDefinition.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Mon Apr 30 08:24:17 2018
New Revision: 331189

URL: http://llvm.org/viewvc/llvm-project?rev=331189=rev
Log:
[clangd] Using index for GoToDefinition.

Summary:
This patch adds index support for GoToDefinition -- when we don't get the
definition from local AST, we query our index (Static) index to
get it.

Since we currently collect top-level symbol in the index, it doesn't support all
cases (e.g. class members), we will extend the index to include more symbols in
the future.

Reviewers: sammccall

Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, MaskRay, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/XRefs.h
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.h
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=331189=331188=331189=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Apr 30 08:24:17 2018
@@ -355,11 +355,11 @@ void ClangdServer::dumpAST(PathRef File,
 void ClangdServer::findDefinitions(PathRef File, Position Pos,
Callback CB) {
   auto FS = FSProvider.getFileSystem();
-  auto Action = [Pos, FS](Callback CB,
-  llvm::Expected InpAST) {
+  auto Action = [Pos, FS, this](Callback CB,
+llvm::Expected InpAST) {
 if (!InpAST)
   return CB(InpAST.takeError());
-CB(clangd::findDefinitions(InpAST->AST, Pos));
+CB(clangd::findDefinitions(InpAST->AST, Pos, this->FileIdx.get()));
   };
 
   WorkScheduler.runWithAST("Definitions", File, Bind(Action, std::move(CB)));

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=331189=331188=331189=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Mon Apr 30 08:24:17 2018
@@ -14,6 +14,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexingAction.h"
+#include "clang/Index/USRGeneration.h"
 #include "llvm/Support/Path.h"
 namespace clang {
 namespace clangd {
@@ -34,6 +35,33 @@ const Decl *GetDefinition(const Decl *D)
   return nullptr;
 }
 
+// Convert a SymbolLocation to LSP's Location.
+// HintPath is used to resolve the path of URI.
+// FIXME: figure out a good home for it, and share the implementation with
+// FindSymbols.
+llvm::Optional ToLSPLocation(const SymbolLocation ,
+   llvm::StringRef HintPath) {
+  if (!Loc)
+return llvm::None;
+  auto Uri = URI::parse(Loc.FileURI);
+  if (!Uri) {
+log("Could not parse URI: " + Loc.FileURI);
+return llvm::None;
+  }
+  auto Path = URI::resolve(*Uri, HintPath);
+  if (!Path) {
+log("Could not resolve URI: " + Loc.FileURI);
+return llvm::None;
+  }
+  Location LSPLoc;
+  LSPLoc.uri = URIForFile(*Path);
+  LSPLoc.range.start.line = Loc.Start.Line;
+  LSPLoc.range.start.character = Loc.Start.Column;
+  LSPLoc.range.end.line = Loc.End.Line;
+  LSPLoc.range.end.character = Loc.End.Column;
+  return LSPLoc;
+}
+
 struct MacroDecl {
   StringRef Name;
   const MacroInfo *Info;
@@ -128,6 +156,38 @@ private:
   }
 };
 
+struct IdentifiedSymbol {
+  std::vector Decls;
+  std::vector Macros;
+};
+
+IdentifiedSymbol getSymbolAtPosition(ParsedAST , SourceLocation Pos) {
+  auto DeclMacrosFinder = DeclarationAndMacrosFinder(
+  llvm::errs(), Pos, AST.getASTContext(), AST.getPreprocessor());
+  index::IndexingOptions IndexOpts;
+  IndexOpts.SystemSymbolFilter =
+  index::IndexingOptions::SystemSymbolFilterKind::All;
+  IndexOpts.IndexFunctionLocals = true;
+  indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
+ DeclMacrosFinder, IndexOpts);
+
+  return {DeclMacrosFinder.takeDecls(), DeclMacrosFinder.takeMacroInfos()};
+}
+
+llvm::Optional
+getAbsoluteFilePath(const FileEntry *F, const SourceManager ) {
+  SmallString<64> FilePath = F->tryGetRealPathName();
+  if (FilePath.empty())
+FilePath = F->getName();
+  if (!llvm::sys::path::is_absolute(FilePath)) {
+if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) {
+  log("Could not turn relative path to absolute: " + FilePath);
+  return llvm::None;
+}
+  }
+  return FilePath.str().str();
+}
+
 llvm::Optional
 makeLocation(ParsedAST , const SourceRange ) {
   const SourceManager  = 

[PATCH] D46218: PR37275 packed attribute should not apply to base classes

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D46218#1081933, @rjmccall wrote:

> I wonder if that was originally just an oversight that got turned into 
> official policy.  Anyway, if it's the policy, it's what we have to do; LGTM.


I think it's actually correct behavior.  Why would an attribute on a derived 
class be allowed to alter the layout of a base class?  It would mean you can't 
convert Derived* to Base* safely.


Repository:
  rL LLVM

https://reviews.llvm.org/D46218



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

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

[Alexander Kornienko via Phabricator via cfe-commits]

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



Comment at: clang-tidy/modernize/UseAutoCheck.cpp:33
+
+bool IsNotSpace(const char& C) {
+  return !std::isspace(static_cast(C));

Why `const char&` and not just `char`? Moreover, these two functions can be 
replaced with lambdas. See below.



Comment at: clang-tidy/modernize/UseAutoCheck.cpp:445-449
+size_t UseAutoCheck::GetTypeNameLength(const SourceRange ,
+   const ASTContext ) {
+  const StringRef S = tooling::fixit::getText(SR, Context);
+  return std::count_if(S.begin(), S.end(), SpacePredicate);
+}

```
static size_t GetTypeNameLength(const TypeLoc , const ASTContext , 
bool IgnoreStars) {
  const StringRef S = tooling::fixit::getText(Loc.getSourceRange(), Context);
  if (IgnoreStars)
return llvm::count_if(S, [] (char C) { return std::isspace(C) || C == '*'; 
});
  return llvm::count_if(S, [] (char C) { return std::isspace(C); });
}
```



Comment at: clang-tidy/modernize/UseAutoCheck.h:31
StringRef Message);
+  size_t GetTypeNameLength(const SourceRange , const ASTContext );
 

I'd make this a static function and remove the `SpacePredicate` field as well. 
See the comment below.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45927



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

[PATCH] D45717: [clangd] Using index for GoToDefinition.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks for the review!




Comment at: clangd/XRefs.cpp:277
+// it.
+auto ToLSPLocation = [](
+const SymbolLocation ) -> llvm::Optional 
{

sammccall wrote:
> hokein wrote:
> > sammccall wrote:
> > > (The double-nested lambda is somewhat hard to read, can this just be a 
> > > top-level function? That's what's needed to share it, right?)
> > Moved it to `XRef.h`, and also replace the one in `findsymbols`.
> This is pretty weird in terms of layering I think :-(
> The function is pretty trivial, but depends on a bunch of random stuff.
> Can we move it back (and live with the duplication) until we come up with a 
> good home?
Reverted it, made it local in this file and added a FIXME.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45717



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

[PATCH] D45717: [clangd] Using index for GoToDefinition.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 144561.
hokein marked 7 inline comments as done.
hokein added a comment.

Address review comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45717

Files:
  clangd/ClangdServer.cpp
  clangd/XRefs.cpp
  clangd/XRefs.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -13,11 +13,14 @@
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "XRefs.h"
+#include "gmock/gmock.h"
+#include "index/FileIndex.h"
+#include "index/SymbolCollector.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Frontend/Utils.h"
+#include "clang/Index/IndexingAction.h"
 #include "llvm/Support/Path.h"
-#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -37,17 +40,33 @@
 };
 
 // FIXME: this is duplicated with FileIndexTests. Share it.
-ParsedAST build(StringRef Code) {
-  auto CI = createInvocationFromCommandLine(
-  {"clang", "-xc++", testPath("Foo.cpp").c_str()});
-  auto Buf = MemoryBuffer::getMemBuffer(Code);
+ParsedAST build(StringRef MainCode, StringRef HeaderCode = "") {
+  auto HeaderPath = testPath("foo.h");
+  auto MainPath = testPath("foo.cpp");
+  llvm::IntrusiveRefCntPtr VFS(
+  new vfs::InMemoryFileSystem());
+  VFS->addFile(MainPath, 0, llvm::MemoryBuffer::getMemBuffer(MainCode));
+  VFS->addFile(HeaderPath, 0, llvm::MemoryBuffer::getMemBuffer(HeaderCode));
+  std::vector Cmd = {"clang", "-xc++", MainPath.c_str()};
+  if (!HeaderCode.empty()) {
+std::vector args = {"-include", HeaderPath.c_str()};
+Cmd.insert(Cmd.begin() + 1, args.begin(), args.end());
+  }
+  auto CI = createInvocationFromCommandLine(Cmd);
+
+  auto Buf = MemoryBuffer::getMemBuffer(MainCode);
   auto AST = ParsedAST::Build(std::move(CI), nullptr, std::move(Buf),
-  std::make_shared(),
-  vfs::getRealFileSystem());
+  std::make_shared(), VFS);
   assert(AST.hasValue());
   return std::move(*AST);
 }
 
+std::unique_ptr buildIndex(StringRef MainCode,
+StringRef HeaderCode) {
+  auto AST = build(MainCode, HeaderCode);
+  return MemIndex::build(indexAST());
+}
+
 // Extracts ranges from an annotated example, and constructs a matcher for a
 // highlight set. Ranges should be named $read/$write as appropriate.
 Matcher &>
@@ -106,6 +125,65 @@
 
 MATCHER_P(RangeIs, R, "") { return arg.range == R; }
 
+TEST(GoToDefinition, WithIndex) {
+  Annotations SymbolHeader(R"cpp(
+class $forward[[Forward]];
+class $foo[[Foo]] {};
+
+void $f1[[f1]]();
+
+inline void $f2[[f2]]() {}
+  )cpp");
+  Annotations SymbolCpp(R"cpp(
+  class $forward[[forward]] {};
+  void  $f1[[f1]]() {}
+)cpp");
+
+  auto Index = buildIndex(SymbolCpp.code(), SymbolHeader.code());
+  auto runFindDefinitionsWithIndex = [](const Annotations ) {
+auto AST = build(/*MainCode=*/Main.code(),
+ /*HeaderCode=*/"");
+return clangd::findDefinitions(AST, Main.point(), Index.get());
+  };
+
+  Annotations Test(R"cpp(// only declaration in AST.
+void [[f1]]();
+int main() {
+  ^f1();
+}
+  )cpp");
+  EXPECT_THAT(runFindDefinitionsWithIndex(Test),
+  testing::ElementsAreArray(
+  {RangeIs(SymbolCpp.range("f1")), RangeIs(Test.range())}));
+
+  Test = Annotations(R"cpp(// definition in AST.
+void [[f1]]() {}
+int main() {
+  ^f1();
+}
+  )cpp");
+  EXPECT_THAT(runFindDefinitionsWithIndex(Test),
+  testing::ElementsAreArray(
+  {RangeIs(Test.range()), RangeIs(SymbolHeader.range("f1"))}));
+
+  Test = Annotations(R"cpp(// forward declaration in AST.
+class [[Foo]];
+F^oo* create();
+  )cpp");
+  EXPECT_THAT(runFindDefinitionsWithIndex(Test),
+  testing::ElementsAreArray(
+  {RangeIs(SymbolHeader.range("foo")), RangeIs(Test.range())}));
+
+  Test = Annotations(R"cpp(// defintion in AST.
+class [[Forward]] {};
+F^orward create();
+  )cpp");
+  EXPECT_THAT(runFindDefinitionsWithIndex(Test),
+  testing::ElementsAreArray({
+  RangeIs(Test.range()), RangeIs(SymbolHeader.range("forward")),
+  }));
+}
+
 TEST(GoToDefinition, All) {
   const char *Tests[] = {
   R"cpp(// Local variable
Index: clangd/index/FileIndex.h
===
--- clangd/index/FileIndex.h
+++ clangd/index/FileIndex.h
@@ -71,6 +71,10 @@
   MemIndex Index;
 };
 
+/// Retrieves namespace and class level symbols in \p AST.
+/// Exposed to assist in unit tests.
+SymbolSlab 

r331177 - IWYU for llvm-config.h in clang. See r331124 for details.

[Nico Weber via cfe-commits]

Author: nico
Date: Mon Apr 30 06:52:15 2018
New Revision: 331177

URL: http://llvm.org/viewvc/llvm-project?rev=331177=rev
Log:
IWYU for llvm-config.h in clang. See r331124 for details.

Modified:
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp
cfe/trunk/lib/Lex/PPMacroExpansion.cpp
cfe/trunk/lib/Tooling/Tooling.cpp
cfe/trunk/tools/driver/cc1_main.cpp
cfe/trunk/tools/driver/driver.cpp
cfe/trunk/tools/libclang/CIndexer.cpp
cfe/trunk/unittests/Basic/FileManagerTest.cpp
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp
cfe/trunk/unittests/Tooling/ToolingTest.cpp

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=331177=331176=331177=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Mon Apr 30 06:52:15 2018
@@ -57,6 +57,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Config/llvm-config.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/OptSpecifier.h"

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=331177=331176=331177=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Mon Apr 30 06:52:15 2018
@@ -30,6 +30,7 @@
 #include "clang/Driver/SanitizerArgs.h"
 #include "clang/Driver/XRayArgs.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Config/llvm-config.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Compression.h"

Modified: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/MSVC.cpp?rev=331177=331176=331177=diff
==
--- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp Mon Apr 30 06:52:15 2018
@@ -19,7 +19,6 @@
 #include "clang/Driver/SanitizerArgs.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
-#include "llvm/Config/llvm-config.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/ConvertUTF.h"

Modified: cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp?rev=331177=331176=331177=diff
==
--- cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp (original)
+++ cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp Mon Apr 30 06:52:15 
2018
@@ -16,6 +16,7 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Serialization/ASTReader.h"
 #include "llvm/ADT/iterator_range.h"
+#include "llvm/Config/llvm-config.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"

Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=331177=331176=331177=diff
==
--- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original)
+++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Mon Apr 30 06:52:15 2018
@@ -41,7 +41,6 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
-#include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"

Modified: cfe/trunk/lib/Tooling/Tooling.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=331177=331176=331177=diff
==
--- cfe/trunk/lib/Tooling/Tooling.cpp (original)
+++ cfe/trunk/lib/Tooling/Tooling.cpp Mon Apr 30 06:52:15 2018
@@ -41,7 +41,6 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
-#include "llvm/Config/llvm-config.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"

Modified: cfe/trunk/tools/driver/cc1_main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1_main.cpp?rev=331177=331176=331177=diff
==
--- cfe/trunk/tools/driver/cc1_main.cpp (original)
+++ cfe/trunk/tools/driver/cc1_main.cpp Mon Apr 30 06:52:15 2018
@@ -26,6 +26,7 @@
 #include 

[PATCH] D45717: [clangd] Using index for GoToDefinition.

[Sam McCall via Phabricator via cfe-commits]

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

A few more nits (and some ideas for further restructuring the merge logic).
Otherwise LG, let's land this!




Comment at: clangd/XRefs.cpp:215
 
-  indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
- DeclMacrosFinder, IndexOpts);
+  // Handle goto definition for macros.
+  if (!Symbols.Macros.empty()) {

hokein wrote:
> sammccall wrote:
> > So now you're early-out if there are macros.
> > This means if you have
> > 
> > ```
> > void log(string s) { cout << s; }
> > #define LOG log
> > LOG("hello");
> > ```
> > 
> > You'll offer only line 2 as an option, and not line 1.
> > I'm not sure that's bad, but it's non-obvious - I think it's the thing that 
> > the comment should call out.
> > e.g. "If the cursor is on a macro, go to the macro's definition without 
> > trying to resolve the code it expands to"
> > (The current comment just echoes the code)
> This is a non-functional change.
> 
> For the above example, only line 2 will be offered. This is expected behavior 
> IMO -- when we go to definition on a macro, users would expect the macro 
> definition.
Ah, so these cases can't both happen (we even have an assert elsewhere).

Still, we can treat them as independent, and it seems simpler: you can remove 
the if statement, the comment for the if statement, and the early return.



Comment at: clangd/XRefs.cpp:237
+  //(e.g. function declaration in header), and a location of definition if
+  //they are available, definition comes first.
+  struct CandidateLocation {

hokein wrote:
> sammccall wrote:
> > first why?
> because this is `go to definition`, so it is sensible to return the 
> `definition` as the first result, right?
Again, I don't think "definition" in LSP is being used in its technical c++ 
sense.
No issue with the behavior here, but I think the comment should be "we think 
that's what the user wants most often" or something - it's good to know the 
reasoning in case we want to change the behavior in the future.



Comment at: clangd/XRefs.cpp:277
+// it.
+auto ToLSPLocation = [](
+const SymbolLocation ) -> llvm::Optional 
{

hokein wrote:
> sammccall wrote:
> > (The double-nested lambda is somewhat hard to read, can this just be a 
> > top-level function? That's what's needed to share it, right?)
> Moved it to `XRef.h`, and also replace the one in `findsymbols`.
This is pretty weird in terms of layering I think :-(
The function is pretty trivial, but depends on a bunch of random stuff.
Can we move it back (and live with the duplication) until we come up with a 
good home?



Comment at: clangd/XRefs.cpp:244
+  //final results. When there are duplicate results, we prefer AST over
+  //index because AST is more up-to-update.
+  //

up-to-update -> up-to-date



Comment at: clangd/XRefs.cpp:257
+  //   4. Return all populated locations for all symbols, definition first.
+  struct CandidateLocation {
+llvm::Optional ID;

OK, as discussed offline the bit that remains confusing is why this isn't a map.
The reason is that some symbols don't have USRs and therefore no symbol IDs.

In practice we don't really expect  multiple symbol results at all, so what 
about one of:
 - `map`
 - `map` and use `SymbolID("")` as a sentinel

I slightly prefer the latter because it minimizes the invasiveness of handling 
this rare/unimportant case.



Comment at: clangd/index/FileIndex.h:26
 
+/// \brief Retrieves namespace and class level symbols in \p AST.
+SymbolSlab indexAST(ParsedAST *AST);

nit: prefer to remove `\brief` in favor of formatting the comment so the 
summary gets extracted.



Comment at: clangd/index/FileIndex.h:26
 
+/// \brief Retrieves namespace and class level symbols in \p AST.
+SymbolSlab indexAST(ParsedAST *AST);

sammccall wrote:
> nit: prefer to remove `\brief` in favor of formatting the comment so the 
> summary gets extracted.
This isn't the main interface of the file.
Move to the bottom, and note that this is exposed to assist in unit tests?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45717



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

r331176 - Remove unnecessary indirection. No behavior change.

[Nico Weber via cfe-commits]

Author: nico
Date: Mon Apr 30 06:47:04 2018
New Revision: 331176

URL: http://llvm.org/viewvc/llvm-project?rev=331176=rev
Log:
Remove unnecessary indirection. No behavior change.

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

Modified: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/MSVC.cpp?rev=331176=331175=331176=diff
==
--- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp Mon Apr 30 06:47:04 2018
@@ -31,13 +31,7 @@
 #include "llvm/Support/Process.h"
 #include 
 
-// Include the necessary headers to interface with the Windows registry and
-// environment.
-#if defined(_WIN32)
-#define USE_WIN32
-#endif
-
-#ifdef USE_WIN32
+#ifdef _WIN32
   #define WIN32_LEAN_AND_MEAN
   #define NOGDI
   #ifndef NOMINMAX
@@ -476,7 +470,7 @@ void visualstudio::Linker::ConstructJob(
 // their own link.exe which may come first.
 linkPath = FindVisualStudioExecutable(TC, "link.exe");
 
-#ifdef USE_WIN32
+#ifdef _WIN32
 // When cross-compiling with VS2017 or newer, link.exe expects to have
 // its containing bin directory at the top of PATH, followed by the
 // native target bin directory.
@@ -841,7 +835,7 @@ MSVCToolChain::getSubDirectoryPath(SubDi
   return Path.str();
 }
 
-#ifdef USE_WIN32
+#ifdef _WIN32
 static bool readFullStringValue(HKEY hkey, const char *valueName,
 std::string ) {
   std::wstring WideValueName;
@@ -885,7 +879,7 @@ static bool readFullStringValue(HKEY hke
 /// characters are compared.  This function only searches HKLM.
 static bool getSystemRegistryString(const char *keyPath, const char *valueName,
 std::string , std::string *phValue) {
-#ifndef USE_WIN32
+#ifndef _WIN32
   return false;
 #else
   HKEY hRootKey = HKEY_LOCAL_MACHINE;
@@ -967,7 +961,7 @@ static bool getSystemRegistryString(cons
 }
   }
   return returnValue;
-#endif // USE_WIN32
+#endif // _WIN32
 }
 
 // Find the most recent version of Universal CRT or Windows 10 SDK.
@@ -1128,7 +1122,7 @@ static VersionTuple getMSVCVersionFromTr
 
 static VersionTuple getMSVCVersionFromExe(const std::string ) {
   VersionTuple Version;
-#ifdef USE_WIN32
+#ifdef _WIN32
   SmallString<128> ClExe(BinDir);
   llvm::sys::path::append(ClExe, "cl.exe");
 


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

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added a comment.

Gentle ping.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45927



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

[PATCH] D46236: [Driver, CodeGen] rename options to disable an FP cast optimization

[Sanjay Patel via Phabricator via cfe-commits]

spatel updated this revision to Diff 144551.
spatel added a comment.

Patch updated:
Added "By default" to the description in the user manual and release notes to 
make it clearer how this behaves.


https://reviews.llvm.org/D46236

Files:
  docs/ReleaseNotes.rst
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGCall.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/no-junk-ftrunc.c
  test/Driver/fast-math.c

Index: test/Driver/fast-math.c
===
--- test/Driver/fast-math.c
+++ test/Driver/fast-math.c
@@ -289,25 +289,25 @@
 // CHECK-NO-TRAPPING-MATH: "-fno-trapping-math"
 
 // This isn't fast-math, but the option is handled in the same place as other FP params.
-// Last option wins, and the flag is *not* passed by default. 
+// Last option wins, and strict behavior is assumed by default. 
 
-// RUN: %clang -### -ffp-cast-overflow-workaround -c %s 2>&1 \
+// RUN: %clang -### -fno-strict-float-cast-overflow -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPOV-WORKAROUND %s
 // CHECK-FPOV-WORKAROUND: "-cc1"
-// CHECK-FPOV-WORKAROUND: "-ffp-cast-overflow-workaround"
+// CHECK-FPOV-WORKAROUND: "-fno-strict-float-cast-overflow"
 
 // RUN: %clang -### -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPOV-WORKAROUND-DEFAULT %s
 // CHECK-FPOV-WORKAROUND-DEFAULT: "-cc1"
-// CHECK-FPOV-WORKAROUND-DEFAULT-NOT: "-ffp-cast-overflow-workaround"
+// CHECK-FPOV-WORKAROUND-DEFAULT-NOT: "strict-float-cast-overflow"
 
-// RUN: %clang -### -fno-fp-cast-overflow-workaround -c %s 2>&1 \
+// RUN: %clang -### -fstrict-float-cast-overflow -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FPOV-WORKAROUND %s
 // CHECK-NO-FPOV-WORKAROUND: "-cc1"
-// CHECK-NO-FPOV-WORKAROUND-NOT: "-ffp-cast-overflow-workaround"
+// CHECK-NO-FPOV-WORKAROUND-NOT: "strict-float-cast-overflow"
 
-// RUN: %clang -### -ffp-cast-overflow-workaround -fno-fp-cast-overflow-workaround -c %s 2>&1 \
+// RUN: %clang -### -fno-strict-float-cast-overflow -fstrict-float-cast-overflow -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FPOV-WORKAROUND-OVERRIDE %s
 // CHECK-NO-FPOV-WORKAROUND-OVERRIDE: "-cc1"
-// CHECK-NO-FPOV-WORKAROUND-OVERRIDE-NOT: "-ffp-cast-overflow-workaround"
+// CHECK-NO-FPOV-WORKAROUND-OVERRIDE-NOT: "strict-float-cast-overflow"
 
Index: test/CodeGen/no-junk-ftrunc.c
===
--- test/CodeGen/no-junk-ftrunc.c
+++ test/CodeGen/no-junk-ftrunc.c
@@ -1,12 +1,16 @@
-// RUN: %clang_cc1 -S -ffp-cast-overflow-workaround %s -emit-llvm -o - | FileCheck %s
-// CHECK-LABEL: main
-// CHECK: attributes #0 = {{.*}}"fp-cast-overflow-workaround"="true"{{.*}}
+// RUN: %clang_cc1 -S -fno-strict-float-cast-overflow %s -emit-llvm -o - | FileCheck %s --check-prefix=NOSTRICT
+// NOSTRICT-LABEL: main
+// NOSTRICT: attributes #0 = {{.*}}"strict-float-cast-overflow"="false"{{.*}}
 
 // The workaround attribute is not applied by default.
 
+// RUN: %clang_cc1 -S %s -fstrict-float-cast-overflow -emit-llvm -o - | FileCheck %s --check-prefix=STRICT
+// STRICT-LABEL: main
+// STRICT-NOT: strict-float-cast-overflow
+
 // RUN: %clang_cc1 -S %s -emit-llvm -o - | FileCheck %s --check-prefix=DEFAULT
 // DEFAULT-LABEL: main
-// DEFAULT-NOT: fp-cast-overflow-workaround
+// DEFAULT-NOT: strict-float-cast-overflow
 
 int main() {
   return 0;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -699,7 +699,8 @@
   Opts.Reciprocals = Args.getAllArgValues(OPT_mrecip_EQ);
   Opts.ReciprocalMath = Args.hasArg(OPT_freciprocal_math);
   Opts.NoTrappingMath = Args.hasArg(OPT_fno_trapping_math);
-  Opts.FPCastOverflowWorkaround = Args.hasArg(OPT_ffp_cast_overflow_workaround);
+  Opts.StrictFloatCastOverflow =
+  !Args.hasArg(OPT_fno_strict_float_cast_overflow);
 
   Opts.NoZeroInitializedInBSS = Args.hasArg(OPT_mno_zero_initialized_in_bss);
   Opts.NumRegisterParameters = getLastArgIntValue(Args, OPT_mregparm, 0, Diags);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2242,9 +2242,9 @@
   }
 
   // Disable a codegen optimization for floating-point casts.
-  if (Args.hasFlag(options::OPT_ffp_cast_overflow_workaround,
-   options::OPT_fno_fp_cast_overflow_workaround, false))
-CmdArgs.push_back("-ffp-cast-overflow-workaround");
+  if (Args.hasFlag(options::OPT_fno_strict_float_cast_overflow,
+   options::OPT_fstrict_float_cast_overflow, false))
+CmdArgs.push_back("-fno-strict-float-cast-overflow");
 }
 
 static void RenderAnalyzerOptions(const ArgList , ArgStringList ,
Index: lib/CodeGen/CGCall.cpp

r331152 - Rename DiagnosticClient to DiagnosticConsumer as per issue 5397.

[Fangrui Song via cfe-commits]

Author: maskray
Date: Sun Apr 29 17:34:09 2018
New Revision: 331152

URL: http://llvm.org/viewvc/llvm-project?rev=331152=rev
Log:
Rename DiagnosticClient to DiagnosticConsumer as per issue 5397.

Modified:
cfe/trunk/docs/InternalsManual.rst
cfe/trunk/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h

Modified: cfe/trunk/docs/InternalsManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/InternalsManual.rst?rev=331152=331151=331152=diff
==
--- cfe/trunk/docs/InternalsManual.rst (original)
+++ cfe/trunk/docs/InternalsManual.rst Sun Apr 29 17:34:09 2018
@@ -54,7 +54,7 @@ number of source ranges that related to
 
 In this section, we'll be giving examples produced by the Clang command line
 driver, but diagnostics can be :ref:`rendered in many different ways
-` depending on how the ``DiagnosticClient`` interface is
+` depending on how the ``DiagnosticConsumer`` interface is
 implemented.  A representative example of a diagnostic is:
 
 .. code-block:: text
@@ -188,7 +188,7 @@ Formatting a Diagnostic Argument
 Arguments to diagnostics are fully typed internally, and come from a couple
 different classes: integers, types, names, and random strings.  Depending on
 the class of the argument, it can be optionally formatted in different ways.
-This gives the ``DiagnosticClient`` information about what the argument means
+This gives the ``DiagnosticConsumer`` information about what the argument means
 without requiring it to use a specific presentation (consider this MVC for
 Clang :).
 
@@ -387,7 +387,7 @@ exactly where those parentheses would be
 fix-it hints themselves describe what changes to make to the source code in an
 abstract manner, which the text diagnostic printer renders as a line of
 "insertions" below the caret line.  :ref:`Other diagnostic clients
-` might choose to render the code differently (e.g., as
+` might choose to render the code differently (e.g., as
 markup inline) or even give the user the ability to automatically fix the
 problem.
 
@@ -420,26 +420,26 @@ Fix-it hints can be created with one of
 Specifies that the code in the given source ``Range`` should be removed,
 and replaced with the given ``Code`` string.
 
-.. _DiagnosticClient:
+.. _DiagnosticConsumer:
 
-The ``DiagnosticClient`` Interface
+The ``DiagnosticConsumer`` Interface
 ^^
 
 Once code generates a diagnostic with all of the arguments and the rest of the
 relevant information, Clang needs to know what to do with it.  As previously
 mentioned, the diagnostic machinery goes through some filtering to map a
 severity onto a diagnostic level, then (assuming the diagnostic is not mapped
-to "``Ignore``") it invokes an object that implements the ``DiagnosticClient``
+to "``Ignore``") it invokes an object that implements the 
``DiagnosticConsumer``
 interface with the information.
 
 It is possible to implement this interface in many different ways.  For
-example, the normal Clang ``DiagnosticClient`` (named
+example, the normal Clang ``DiagnosticConsumer`` (named
 ``TextDiagnosticPrinter``) turns the arguments into strings (according to the
 various formatting rules), prints out the file/line/column information and the
 string, then prints out the line of code, the source ranges, and the caret.
 However, this behavior isn't required.
 
-Another implementation of the ``DiagnosticClient`` interface is the
+Another implementation of the ``DiagnosticConsumer`` interface is the
 ``TextDiagnosticBuffer`` class, which is used when Clang is in ``-verify``
 mode.  Instead of formatting and printing out the diagnostics, this
 implementation just captures and remembers the diagnostics as they fly by.

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h?rev=331152=331151=331152=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h Sun 
Apr 29 17:34:09 2018
@@ -1,4 +1,4 @@
-//===--- PathDiagnosticClients.h - Path Diagnostic Clients --*- C++ 
-*-===//
+//===--- PathDiagnosticConsumers.h - Path Diagnostic Clients --*- C++ 
-*-===//
 //
 // The LLVM Compiler Infrastructure
 //


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

[PATCH] D46258: [clangd] Also use UTF-16 in index position.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL331168: [clangd] Also use UTF-16 in index position. 
(authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D46258

Files:
  clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
  clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
@@ -689,6 +689,15 @@
IncludeHeader(TestHeaderURI), 
DefURI(TestFileURI;
 }
 
+TEST_F(SymbolCollectorTest, UTF16Character) {
+  // ö is 2-bytes.
+  Annotations Header(/*Header=*/"class [[pörk]] {};");
+  runSymbolCollector(Header.code(), /*Main=*/"");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+   AllOf(QName("pörk"), DeclRange(Header.range();
+}
+
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
@@ -195,14 +195,10 @@
   auto TokenLength = clang::Lexer::MeasureTokenLength(NameLoc, SM, LangOpts);
 
   auto CreatePosition = [](SourceLocation Loc) {
-auto FileIdAndOffset = SM.getDecomposedLoc(Loc);
-auto FileId = FileIdAndOffset.first;
-auto Offset = FileIdAndOffset.second;
+auto LSPLoc = sourceLocToPosition(SM, Loc);
 SymbolLocation::Position Pos;
-// Position is 0-based while SourceManager is 1-based.
-Pos.Line = SM.getLineNumber(FileId, Offset) - 1;
-// FIXME: Use UTF-16 code units, not UTF-8 bytes.
-Pos.Column = SM.getColumnNumber(FileId, Offset) - 1;
+Pos.Line = LSPLoc.line;
+Pos.Column = LSPLoc.character;
 return Pos;
   };
 


Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
@@ -689,6 +689,15 @@
IncludeHeader(TestHeaderURI), DefURI(TestFileURI;
 }
 
+TEST_F(SymbolCollectorTest, UTF16Character) {
+  // ö is 2-bytes.
+  Annotations Header(/*Header=*/"class [[pörk]] {};");
+  runSymbolCollector(Header.code(), /*Main=*/"");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+   AllOf(QName("pörk"), DeclRange(Header.range();
+}
+
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
@@ -195,14 +195,10 @@
   auto TokenLength = clang::Lexer::MeasureTokenLength(NameLoc, SM, LangOpts);
 
   auto CreatePosition = [](SourceLocation Loc) {
-auto FileIdAndOffset = SM.getDecomposedLoc(Loc);
-auto FileId = FileIdAndOffset.first;
-auto Offset = FileIdAndOffset.second;
+auto LSPLoc = sourceLocToPosition(SM, Loc);
 SymbolLocation::Position Pos;
-// Position is 0-based while SourceManager is 1-based.
-Pos.Line = SM.getLineNumber(FileId, Offset) - 1;
-// FIXME: Use UTF-16 code units, not UTF-8 bytes.
-Pos.Column = SM.getColumnNumber(FileId, Offset) - 1;
+Pos.Line = LSPLoc.line;
+Pos.Column = LSPLoc.character;
 return Pos;
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r331168 - [clangd] Also use UTF-16 in index position.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Mon Apr 30 04:40:02 2018
New Revision: 331168

URL: http://llvm.org/viewvc/llvm-project?rev=331168=rev
Log:
[clangd] Also use UTF-16 in index position.

Reviewers: sammccall

Subscribers: klimek, ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=331168=331167=331168=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Mon Apr 30 
04:40:02 2018
@@ -195,14 +195,10 @@ llvm::Optional getSymbol
   auto TokenLength = clang::Lexer::MeasureTokenLength(NameLoc, SM, LangOpts);
 
   auto CreatePosition = [](SourceLocation Loc) {
-auto FileIdAndOffset = SM.getDecomposedLoc(Loc);
-auto FileId = FileIdAndOffset.first;
-auto Offset = FileIdAndOffset.second;
+auto LSPLoc = sourceLocToPosition(SM, Loc);
 SymbolLocation::Position Pos;
-// Position is 0-based while SourceManager is 1-based.
-Pos.Line = SM.getLineNumber(FileId, Offset) - 1;
-// FIXME: Use UTF-16 code units, not UTF-8 bytes.
-Pos.Column = SM.getColumnNumber(FileId, Offset) - 1;
+Pos.Line = LSPLoc.line;
+Pos.Column = LSPLoc.character;
 return Pos;
   };
 

Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=331168=331167=331168=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Mon Apr 
30 04:40:02 2018
@@ -689,6 +689,15 @@ TEST_F(SymbolCollectorTest, ClassForward
IncludeHeader(TestHeaderURI), 
DefURI(TestFileURI;
 }
 
+TEST_F(SymbolCollectorTest, UTF16Character) {
+  // ö is 2-bytes.
+  Annotations Header(/*Header=*/"class [[pörk]] {};");
+  runSymbolCollector(Header.code(), /*Main=*/"");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+   AllOf(QName("pörk"), DeclRange(Header.range();
+}
+
+
 } // namespace
 } // namespace clangd
 } // namespace clang


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

[PATCH] D46258: [clangd] Also use UTF-16 in index position.

[Haojian Wu via Phabricator via cfe-commits]

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

Add a test.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46258

Files:
  clangd/index/SymbolCollector.cpp
  unittests/clangd/SymbolCollectorTests.cpp


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -689,6 +689,15 @@
IncludeHeader(TestHeaderURI), 
DefURI(TestFileURI;
 }
 
+TEST_F(SymbolCollectorTest, UTF16Character) {
+  // ö is 2-bytes.
+  Annotations Header(/*Header=*/"class [[pörk]] {};");
+  runSymbolCollector(Header.code(), /*Main=*/"");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+   AllOf(QName("pörk"), DeclRange(Header.range();
+}
+
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -195,14 +195,10 @@
   auto TokenLength = clang::Lexer::MeasureTokenLength(NameLoc, SM, LangOpts);
 
   auto CreatePosition = [](SourceLocation Loc) {
-auto FileIdAndOffset = SM.getDecomposedLoc(Loc);
-auto FileId = FileIdAndOffset.first;
-auto Offset = FileIdAndOffset.second;
+auto LSPLoc = sourceLocToPosition(SM, Loc);
 SymbolLocation::Position Pos;
-// Position is 0-based while SourceManager is 1-based.
-Pos.Line = SM.getLineNumber(FileId, Offset) - 1;
-// FIXME: Use UTF-16 code units, not UTF-8 bytes.
-Pos.Column = SM.getColumnNumber(FileId, Offset) - 1;
+Pos.Line = LSPLoc.line;
+Pos.Column = LSPLoc.character;
 return Pos;
   };
 


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -689,6 +689,15 @@
IncludeHeader(TestHeaderURI), DefURI(TestFileURI;
 }
 
+TEST_F(SymbolCollectorTest, UTF16Character) {
+  // ö is 2-bytes.
+  Annotations Header(/*Header=*/"class [[pörk]] {};");
+  runSymbolCollector(Header.code(), /*Main=*/"");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+   AllOf(QName("pörk"), DeclRange(Header.range();
+}
+
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -195,14 +195,10 @@
   auto TokenLength = clang::Lexer::MeasureTokenLength(NameLoc, SM, LangOpts);
 
   auto CreatePosition = [](SourceLocation Loc) {
-auto FileIdAndOffset = SM.getDecomposedLoc(Loc);
-auto FileId = FileIdAndOffset.first;
-auto Offset = FileIdAndOffset.second;
+auto LSPLoc = sourceLocToPosition(SM, Loc);
 SymbolLocation::Position Pos;
-// Position is 0-based while SourceManager is 1-based.
-Pos.Line = SM.getLineNumber(FileId, Offset) - 1;
-// FIXME: Use UTF-16 code units, not UTF-8 bytes.
-Pos.Column = SM.getColumnNumber(FileId, Offset) - 1;
+Pos.Line = LSPLoc.line;
+Pos.Column = LSPLoc.character;
 return Pos;
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46190: For an ODR declaration, set the 'Used' bit on its associated declarations.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso added a comment.

My initial approach was based on the following test case:

  void Bar() {
typedef int I_Ref;
I_Ref var_bar;
  }
  
  void Foo() {
typedef int I_Used;
I_Used var_foo;
var_foo = 2;
  }
  
  void Test() {
Foo();
  }

and the generated AST looks like:

  |-FunctionDecl Bar 'void ()'
  |   |-DeclStmt
  |   | `-TypedefDecl referenced I_Ref 'int'
  |   |-DeclStmt
  | `-VarDecl var_bar 'I_Ref':'int'
  
  |-FunctionDecl used Foo 'void ()'
  |   |-DeclStmt
  |   | `-TypedefDecl referenced I_Used 'int'
  |   |-DeclStmt
  |   | `-VarDecl used var_foo 'I_Used':'int'

The typedef 'I_Ref' is marked as 'referenced' due to its association with 
'var_bar'.
The typedef 'I_Used' is marked as 'referenced' despite that its association 
with 'var_foo' which is 'used'
Both 'typedefs' are marked as 'referenced'.

With the intended patch, the AST looks like:

  |-FunctionDecl Bar 'void ()'
  |   |-DeclStmt
  |   | `-TypedefDecl referenced I_Ref 'int'
  |   `-DeclStmt
  | `-VarDecl var_bar 'I_Ref':'int'
  
  |-FunctionDecl used Foo 'void ()'
  |   |-DeclStmt
  |   | `-TypedefDecl used I_Used 'int'
  |   |-DeclStmt
  |   | `-VarDecl used var_foo 'I_Used':'int'

The typedef 'I_Ref' is marked as 'referenced'.
The typedef 'I_Used' is marked as 'used'.


Repository:
  rC Clang

https://reviews.llvm.org/D46190



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

[PATCH] D44684: [mips] Improve handling of -fno-[pic/PIC] option

[Simon Dardis via Phabricator via cfe-commits]

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

LGTM, with a touch up of the error message to match the others we have 
regarding -mabicalls. Some other minor nits inlined.




Comment at: include/clang/Basic/DiagnosticDriverKinds.td:340
+  "ignoring '%0' option as it cannot be used with "
+  "explicit use of -mabicalls and the N64 ABI">,
   InGroup;

abeserminji wrote:
> sdardis wrote:
> > Use the %select{optionA|optionB|..|optionZ}$NUM operator here like:
> > 
> > "cannot be used with the %select{explicit|implicit}1 usage of -mabicalls 
> > and the N64 ABI"
> > 
> > Which eliminates the need for two separate warnings.
> Thanks for the hint. I simplified it now.
Just saw the error messages above; so for style, you should reformat the 
message to be of the form "ignoring '%0' option as it cannt be used with 
%select{|implicit usage of}1-mabicalls and the N64 ABI".

This way all the error messages are consistent. 



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:1024
+mips::getMipsCPUAndABI(Args, Triple, CPUName, ABIName);
+// For n64 ABI, PIC is always true, except in case when -mno-abicalls
+// option is used. And we exit at next check if that's true,

Nit: "When targeting the N64 ABI, PIC is the default, except in the case when 
the -mno-abicalls option is used."



Comment at: test/Driver/mips-abicalls-warning.c:2
 // REQUIRES: mips-registered-target
-// RUN: %clang -### -c -target mips64-mti-elf -fno-PIC -mabicalls %s 2>&1 | 
FileCheck %s
-// CHECK: warning: ignoring '-mabicalls' option as it cannot be used with non 
position-independent code and the N64 ABI
+// RUN: %clang -### -c -target mips64-mti-elf -fno-pic %s 2>&1 | FileCheck 
-check-prefix=CHECK-PIC1-IMPLICIT %s
+// CHECK-PIC1-IMPLICIT: warning: ignoring '-fno-pic' option as it cannot be 
used with implicit use of -mabicalls and the N64 ABI

Add some checks for -fno-pie,  -fno-PIE.


https://reviews.llvm.org/D44684



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

[PATCH] D46258: [clangd] Also use UTF-16 in index position.

[Sam McCall via Phabricator via cfe-commits]

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

(oops, meant to accept this - please do add a test if you can)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46258



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

[PATCH] D46258: [clangd] Also use UTF-16 in index position.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Oops, I missed this, thanks!
Possible to add a testcase? Any character that's not ASCII and is legal in an 
identifier should do it.
http://en.cppreference.com/w/cpp/language/identifiers


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46258



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

[PATCH] D46258: [clangd] Also use UTF-16 in index position.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: jkorous, MaskRay, ioeric, ilya-biryukov, klimek.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46258

Files:
  clangd/index/SymbolCollector.cpp


Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -195,14 +195,10 @@
   auto TokenLength = clang::Lexer::MeasureTokenLength(NameLoc, SM, LangOpts);
 
   auto CreatePosition = [](SourceLocation Loc) {
-auto FileIdAndOffset = SM.getDecomposedLoc(Loc);
-auto FileId = FileIdAndOffset.first;
-auto Offset = FileIdAndOffset.second;
+auto LSPLoc = sourceLocToPosition(SM, Loc);
 SymbolLocation::Position Pos;
-// Position is 0-based while SourceManager is 1-based.
-Pos.Line = SM.getLineNumber(FileId, Offset) - 1;
-// FIXME: Use UTF-16 code units, not UTF-8 bytes.
-Pos.Column = SM.getColumnNumber(FileId, Offset) - 1;
+Pos.Line = LSPLoc.line;
+Pos.Column = LSPLoc.character;
 return Pos;
   };
 


Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -195,14 +195,10 @@
   auto TokenLength = clang::Lexer::MeasureTokenLength(NameLoc, SM, LangOpts);
 
   auto CreatePosition = [](SourceLocation Loc) {
-auto FileIdAndOffset = SM.getDecomposedLoc(Loc);
-auto FileId = FileIdAndOffset.first;
-auto Offset = FileIdAndOffset.second;
+auto LSPLoc = sourceLocToPosition(SM, Loc);
 SymbolLocation::Position Pos;
-// Position is 0-based while SourceManager is 1-based.
-Pos.Line = SM.getLineNumber(FileId, Offset) - 1;
-// FIXME: Use UTF-16 code units, not UTF-8 bytes.
-Pos.Column = SM.getColumnNumber(FileId, Offset) - 1;
+Pos.Line = LSPLoc.line;
+Pos.Column = LSPLoc.character;
 return Pos;
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46176: Pull helper class Environment from clangFormat into libToolingCore [NFC]

[Eric Liu via Phabricator via cfe-commits]

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

- Revert last revision.


Repository:
  rC Clang

https://reviews.llvm.org/D46176

Files:
  include/clang/Tooling/Core/Environment.h
  lib/Format/Format.cpp
  lib/Format/SortJavaScriptImports.cpp
  lib/Format/TokenAnalyzer.cpp
  lib/Format/TokenAnalyzer.h
  lib/Tooling/Core/CMakeLists.txt
  lib/Tooling/Core/Environment.cpp

Index: lib/Tooling/Core/Environment.cpp
===
--- /dev/null
+++ lib/Tooling/Core/Environment.cpp
@@ -0,0 +1,47 @@
+//===--- Environment.cpp - Sourece tooling environment --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Tooling/Core/Environment.h"
+
+namespace clang {
+namespace tooling {
+
+std::unique_ptr
+Environment::createVirtualEnvironment(StringRef Content, StringRef FileName) {
+  // This is referenced by `FileMgr` and will be released by `FileMgr` when it
+  // is deleted.
+  IntrusiveRefCntPtr InMemoryFileSystem(
+  new vfs::InMemoryFileSystem);
+  // This is passed to `SM` as reference, so the pointer has to be referenced
+  // in `Environment` so that `FileMgr` can out-live this function scope.
+  std::unique_ptr FileMgr(
+  new FileManager(FileSystemOptions(), InMemoryFileSystem));
+  // This is passed to `SM` as reference, so the pointer has to be referenced
+  // by `Environment` due to the same reason above.
+  std::unique_ptr Diagnostics(new DiagnosticsEngine(
+  IntrusiveRefCntPtr(new DiagnosticIDs),
+  new DiagnosticOptions));
+  // This will be stored as reference, so the pointer has to be stored in
+  // due to the same reason above.
+  std::unique_ptr VirtualSM(
+  new SourceManager(*Diagnostics, *FileMgr));
+  InMemoryFileSystem->addFile(
+  FileName, 0,
+  llvm::MemoryBuffer::getMemBuffer(Content, FileName,
+   /*RequiresNullTerminator=*/false));
+  FileID ID = VirtualSM->createFileID(FileMgr->getFile(FileName),
+  SourceLocation(), clang::SrcMgr::C_User);
+  assert(ID.isValid());
+
+  return llvm::make_unique(
+  ID, std::move(FileMgr), std::move(VirtualSM), std::move(Diagnostics));
+}
+
+} // namespace tooling
+} // namespace clang
Index: lib/Tooling/Core/CMakeLists.txt
===
--- lib/Tooling/Core/CMakeLists.txt
+++ lib/Tooling/Core/CMakeLists.txt
@@ -1,9 +1,10 @@
 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangToolingCore
+  Diagnostic.cpp
+  Environment.cpp
   Lookup.cpp
   Replacement.cpp
-  Diagnostic.cpp
 
   LINK_LIBS
   clangAST
Index: lib/Format/TokenAnalyzer.h
===
--- lib/Format/TokenAnalyzer.h
+++ lib/Format/TokenAnalyzer.h
@@ -28,6 +28,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
+#include "clang/Tooling/Core/Environment.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 
@@ -37,43 +38,26 @@
 class Environment {
 public:
   Environment(SourceManager , FileID ID, ArrayRef Ranges)
-  : ID(ID), CharRanges(Ranges.begin(), Ranges.end()), SM(SM),
-  FirstStartColumn(0),
-  NextStartColumn(0),
-  LastStartColumn(0) {}
-
-  Environment(FileID ID, std::unique_ptr FileMgr,
-  std::unique_ptr VirtualSM,
-  std::unique_ptr Diagnostics,
-  const std::vector ,
-  unsigned FirstStartColumn,
-  unsigned NextStartColumn,
-  unsigned LastStartColumn)
-  : ID(ID), CharRanges(CharRanges.begin(), CharRanges.end()),
-SM(*VirtualSM), 
-FirstStartColumn(FirstStartColumn),
-NextStartColumn(NextStartColumn),
-LastStartColumn(LastStartColumn),
-FileMgr(std::move(FileMgr)),
-VirtualSM(std::move(VirtualSM)), Diagnostics(std::move(Diagnostics)) {}
+  : ToolEnv(new tooling::Environment(SM, ID)),
+CharRanges(Ranges.begin(), Ranges.end()), FirstStartColumn(0),
+NextStartColumn(0), LastStartColumn(0) {}
 
   // This sets up an virtual file system with file \p FileName containing the
   // fragment \p Code. Assumes that \p Code starts at \p FirstStartColumn,
   // that the next lines of \p Code should start at \p NextStartColumn, and
   // that \p Code should end at \p LastStartColumn if it ends in newline.
   // See also the documentation of clang::format::internal::reformat.
-  static std::unique_ptr
-  CreateVirtualEnvironment(StringRef Code, StringRef FileName,
-   ArrayRef Ranges,
-   unsigned FirstStartColumn = 0,
-   unsigned 

[PATCH] D46190: For an ODR declaration, set the 'Used' bit on its associated declarations.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D46190#1081836, @rsmith wrote:

> In https://reviews.llvm.org/D46190#1081773, @rsmith wrote:
>
> > No, absolutely not. This would be insanely expensive, and marks entirely 
> > unrelated things "used".
>
>
> My apologies, reading this again it comes off a lot harsher than I'd intended.


I appreciate your apology and I understand your concerns. I do not have much 
knowledge on this area.

My initial intention was to traverse just the associated type for an odr-used 
declaration and mark them as used, as an indication that those types are 
"really needed" for that odr-declaration.  The type chain is traversed just 
once.

The approach was intended as a general solution, not only for the original 
'using declaration' issue.

I will have a look at your suggested approach.

Thanks very much for your feedback.


Repository:
  rC Clang

https://reviews.llvm.org/D46190



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

r331164 - [Targets] Implement getConstraintRegister for ARM and AArch64

[Mikhail Maltsev via cfe-commits]

Author: miyuki
Date: Mon Apr 30 02:11:08 2018
New Revision: 331164

URL: http://llvm.org/viewvc/llvm-project?rev=331164=rev
Log:
[Targets] Implement getConstraintRegister for ARM and AArch64

Summary:
The getConstraintRegister method is used by semantic checking of
inline assembly statements in order to diagnose conflicts between
clobber list and input/output lists. Currently ARM and AArch64 don't
override getConstraintRegister, so conflicts between registers
assigned to variables in asm labels and clobber lists are not
diagnosed. Such conflicts can cause assertion failures in the back end
and even miscompilations.

This patch implements getConstraintRegister for ARM and AArch64
targets. Since these targets don't have single-register constraints,
the implementation is trivial and just returns the register specified
in an asm label (if any).

Reviewers: eli.friedman, javed.absar, thopre

Reviewed By: thopre

Subscribers: rengolin, eraman, rogfer01, myatsina, kristof.beyls, cfe-commits, 
chrib

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

Modified:
cfe/trunk/include/clang/Basic/TargetInfo.h
cfe/trunk/lib/Basic/Targets/AArch64.h
cfe/trunk/lib/Basic/Targets/ARM.h
cfe/trunk/test/Sema/arm-asm.c
cfe/trunk/test/Sema/arm64-inline-asm.c

Modified: cfe/trunk/include/clang/Basic/TargetInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=331164=331163=331164=diff
==
--- cfe/trunk/include/clang/Basic/TargetInfo.h (original)
+++ cfe/trunk/include/clang/Basic/TargetInfo.h Mon Apr 30 02:11:08 2018
@@ -629,6 +629,12 @@ public:
   StringRef getNormalizedGCCRegisterName(StringRef Name,
  bool ReturnCanonical = false) const;
 
+  /// \brief Extracts a register from the passed constraint (if it is a
+  /// single-register constraint) and the asm label expression related to a
+  /// variable in the input or output list of an inline asm statement.
+  ///
+  /// This function is used by Sema in order to diagnose conflicts between
+  /// the clobber list and the input/output lists.
   virtual StringRef getConstraintRegister(StringRef Constraint,
   StringRef Expression) const {
 return "";

Modified: cfe/trunk/lib/Basic/Targets/AArch64.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AArch64.h?rev=331164=331163=331164=diff
==
--- cfe/trunk/lib/Basic/Targets/AArch64.h (original)
+++ cfe/trunk/lib/Basic/Targets/AArch64.h Mon Apr 30 02:11:08 2018
@@ -82,6 +82,11 @@ public:
  std::string ) const override;
   const char *getClobbers() const override;
 
+  StringRef getConstraintRegister(StringRef Constraint,
+  StringRef Expression) const override {
+return Expression;
+  }
+
   int getEHDataRegisterNumber(unsigned RegNo) const override;
 };
 

Modified: cfe/trunk/lib/Basic/Targets/ARM.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/ARM.h?rev=331164=331163=331164=diff
==
--- cfe/trunk/lib/Basic/Targets/ARM.h (original)
+++ cfe/trunk/lib/Basic/Targets/ARM.h Mon Apr 30 02:11:08 2018
@@ -156,6 +156,11 @@ public:
  std::string ) const override;
   const char *getClobbers() const override;
 
+  StringRef getConstraintRegister(StringRef Constraint,
+  StringRef Expression) const override {
+return Expression;
+  }
+
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override;
 
   int getEHDataRegisterNumber(unsigned RegNo) const override;

Modified: cfe/trunk/test/Sema/arm-asm.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/arm-asm.c?rev=331164=331163=331164=diff
==
--- cfe/trunk/test/Sema/arm-asm.c (original)
+++ cfe/trunk/test/Sema/arm-asm.c Mon Apr 30 02:11:08 2018
@@ -10,3 +10,10 @@ void test_64bit_r(void) {
   long long foo = 0, bar = 0;
   asm volatile("INST %0, %1" : "=r"(foo) : "r"(bar));
 }
+
+void test_clobber_conflict(void) {
+  register int x asm("r1");
+  asm volatile("nop" :: "r"(x) : "%r1"); // expected-error {{conflicts with 
asm clobber list}}
+  asm volatile("nop" :: "l"(x) : "%r1"); // expected-error {{conflicts with 
asm clobber list}}
+  asm volatile("nop" : "=r"(x) :: "%r1"); // expected-error {{conflicts with 
asm clobber list}}
+}

Modified: cfe/trunk/test/Sema/arm64-inline-asm.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/arm64-inline-asm.c?rev=331164=331163=331164=diff
==
--- cfe/trunk/test/Sema/arm64-inline-asm.c (original)
+++ 

[PATCH] D45965: [Targets] Implement getConstraintRegister for ARM and AArch64

[Mikhail Maltsev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC331164: [Targets] Implement getConstraintRegister for ARM 
and AArch64 (authored by miyuki, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D45965

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/Targets/AArch64.h
  lib/Basic/Targets/ARM.h
  test/Sema/arm-asm.c
  test/Sema/arm64-inline-asm.c


Index: include/clang/Basic/TargetInfo.h
===
--- include/clang/Basic/TargetInfo.h
+++ include/clang/Basic/TargetInfo.h
@@ -629,6 +629,12 @@
   StringRef getNormalizedGCCRegisterName(StringRef Name,
  bool ReturnCanonical = false) const;
 
+  /// \brief Extracts a register from the passed constraint (if it is a
+  /// single-register constraint) and the asm label expression related to a
+  /// variable in the input or output list of an inline asm statement.
+  ///
+  /// This function is used by Sema in order to diagnose conflicts between
+  /// the clobber list and the input/output lists.
   virtual StringRef getConstraintRegister(StringRef Constraint,
   StringRef Expression) const {
 return "";
Index: test/Sema/arm64-inline-asm.c
===
--- test/Sema/arm64-inline-asm.c
+++ test/Sema/arm64-inline-asm.c
@@ -7,3 +7,9 @@
 
   asm volatile("USE(%0)" :: "z"(0)); // expected-warning {{value size does not 
match register size specified by the constraint and modifier}} expected-note 
{{use constraint modifier "w"}}
 }
+
+void test_clobber_conflict(void) {
+  register long x asm("x1");
+  asm volatile("nop" :: "r"(x) : "%x1"); // expected-error {{conflicts with 
asm clobber list}}
+  asm volatile("nop" : "=r"(x) :: "%x1"); // expected-error {{conflicts with 
asm clobber list}}
+}
Index: test/Sema/arm-asm.c
===
--- test/Sema/arm-asm.c
+++ test/Sema/arm-asm.c
@@ -10,3 +10,10 @@
   long long foo = 0, bar = 0;
   asm volatile("INST %0, %1" : "=r"(foo) : "r"(bar));
 }
+
+void test_clobber_conflict(void) {
+  register int x asm("r1");
+  asm volatile("nop" :: "r"(x) : "%r1"); // expected-error {{conflicts with 
asm clobber list}}
+  asm volatile("nop" :: "l"(x) : "%r1"); // expected-error {{conflicts with 
asm clobber list}}
+  asm volatile("nop" : "=r"(x) :: "%r1"); // expected-error {{conflicts with 
asm clobber list}}
+}
Index: lib/Basic/Targets/AArch64.h
===
--- lib/Basic/Targets/AArch64.h
+++ lib/Basic/Targets/AArch64.h
@@ -82,6 +82,11 @@
  std::string ) const override;
   const char *getClobbers() const override;
 
+  StringRef getConstraintRegister(StringRef Constraint,
+  StringRef Expression) const override {
+return Expression;
+  }
+
   int getEHDataRegisterNumber(unsigned RegNo) const override;
 };
 
Index: lib/Basic/Targets/ARM.h
===
--- lib/Basic/Targets/ARM.h
+++ lib/Basic/Targets/ARM.h
@@ -156,6 +156,11 @@
  std::string ) const override;
   const char *getClobbers() const override;
 
+  StringRef getConstraintRegister(StringRef Constraint,
+  StringRef Expression) const override {
+return Expression;
+  }
+
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override;
 
   int getEHDataRegisterNumber(unsigned RegNo) const override;


Index: include/clang/Basic/TargetInfo.h
===
--- include/clang/Basic/TargetInfo.h
+++ include/clang/Basic/TargetInfo.h
@@ -629,6 +629,12 @@
   StringRef getNormalizedGCCRegisterName(StringRef Name,
  bool ReturnCanonical = false) const;
 
+  /// \brief Extracts a register from the passed constraint (if it is a
+  /// single-register constraint) and the asm label expression related to a
+  /// variable in the input or output list of an inline asm statement.
+  ///
+  /// This function is used by Sema in order to diagnose conflicts between
+  /// the clobber list and the input/output lists.
   virtual StringRef getConstraintRegister(StringRef Constraint,
   StringRef Expression) const {
 return "";
Index: test/Sema/arm64-inline-asm.c
===
--- test/Sema/arm64-inline-asm.c
+++ test/Sema/arm64-inline-asm.c
@@ -7,3 +7,9 @@
 
   asm volatile("USE(%0)" :: "z"(0)); // expected-warning {{value size does not match register size specified by the constraint and modifier}} expected-note {{use constraint modifier "w"}}
 }
+
+void test_clobber_conflict(void) {
+  register long x asm("x1");
+  asm volatile("nop" :: "r"(x) : "%x1"); // 

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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I will migrate to the new API today evening (european time).

Why do you think that looping is required? From my understanding, we
need the first usage (DeclRefExpr) to get the analysis started. The
analysis itself will check all remaining uses. Is this necessary,
because we analysis on a `Expr` basis?

Am 29.04.2018 um 20:19 schrieb Shuai Wang via Phabricator:

> shuaiwang added inline comments.
> 
> 
>  Comment at: clang-tidy/cppcoreguidelines/ConstCheck.cpp:229-237
>  +  const auto *UseExpr = selectFirst("use", Usage);
>  +
>  +  // The declared variables was used in non-const conserving way and can not
>  +  // be declared as const.
>  +  if (UseExpr && Scopes[Scope]->isMutated(UseExpr)) {
>  +// diag(UseExpr->getLocStart(), "Investigating starting with this use",
>  +// DiagnosticIDs::Note);
> 
>  
> 
> I think we need to loop over usages instead of just checking the first, i.e.:
> 
>   for (const auto  : Usage) {
> const auto* UseExpr = Nodes.getNodeAs("use");
> if (UseExpr && isMutated(UseExpr)) return true;
>   }
> 
> 
> I'll add a helper function in the MutationAnalyzer for checking varDecl 
> directly as well per your comment there, which you'll be able to use directly 
> in this check. Before that's ready (and if you have time of course) could you 
> help check how many false positives are left with this change?
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D45444


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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

[PATCH] D46035: [clangd] Fix unicode handling, using UTF-16 where LSP requires it.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/trunk/clangd/SourceCode.cpp:38
+// 0xxx is ASCII, handled above. 10xxx is a trailing byte, invalid here.
+// 1xxx is not valid UTF-8 at all. Assert because it's probably our 
bug.
+assert((UTF8Length >= 2 && UTF8Length <= 4) &&

benhamilton wrote:
> This is user input, right? Have we actually checked for valid UTF-8, or do we 
> just assume it's valid?
> 
> If not, it seems like an assertion is not the right check, but we should 
> reject it when we're reading the input.
> 
Yeah, I wasn't sure about this, offline discussion tentatively concluded we 
wanted an assert, but I'm happy to switch to something else.

We don't validate the code on the way in, so strings are "bytes of 
presumed-UTF8". This is usually not a big pain actually. But we could/should 
certainly make the JSON parser validate the UTF-8. (If we want to go this 
route, D45753 should be resolved first).

There's two ways the assertion could fire: the code is invalid UTF-8, or 
there's a bug in the unicode logic here. I thought the latter was more likely 
at least in the short-term :) and this is the least invasive way to catch it. 
And if a developer build (assert-enabled) crashes because an editor feeds it 
invalid bytes, then that's probably better than doing nothing (though not as 
good as catching the error earlier).


Repository:
  rL LLVM

https://reviews.llvm.org/D46035



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

[PATCH] D45814: Fix an assertion when -print-prog-name=

[Christian Bruel via Phabricator via cfe-commits]

chrib added a comment.

In https://reviews.llvm.org/D45814#1081203, @compnerd wrote:

> Do you have commit access or do you need someone to commit this on your 
> behalf?


can you commit it for me please ? thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D45814



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