Re: [cfe-dev] r347339 - [clang][Parse] Diagnose useless null statements / empty init-statements

[Roman Lebedev via cfe-commits]

On Wed, Dec 5, 2018 at 4:07 AM Artem Dergachev via cfe-commits
 wrote:
>
>
>
> On 12/4/18 5:04 PM, George Karpenkov via cfe-dev wrote:
> >
> >> On Dec 4, 2018, at 4:47 PM, Aaron Ballman  wrote:
> >>
> >> On Tue, Dec 4, 2018 at 7:35 PM George Karpenkov  
> >> wrote:
> >>> Hi Roman,
Hi.

> >>> I’m against this new warning.
> >>>
> >>> A very common idiom is defining a “DEBUG” macro to be a no-op in release, 
> >>> and a logging function otherwise.
> >>> The new introduced warning warns on all such cases (e.g. 
> >>> 'DEBUG(“mystring”);')
> >> Yeah, that's not good.
It does warn on such cases when the macro not expanded to nothing, yes.

> >>> As noted in the review, Clang warnings should generally not be used to 
> >>> diagnose style issues — and an extra semicolon does not seem to be a 
> >>> common bug source.
I don't disagree.
The alternative solution would be to try to squeeze this bit of info
into the AST,
without increasing the size of the particular type, and then emit the
diag in clang-tidy.
I did consider it, it seemed like a over-engineering.

> >>> It is a problem in practice for us since we have projects which enable 
> >>> all available warnings and also enable “-Werror”.
Hm, they have asked for -Weverything and they got it. Seems to be
working as intended.
When in previous discussions elsewhere i have talked about
-Weverything, i was always
been told that it isn't supposed to be used like that (admittedly, i
*do* use it like that :)).

Could you explain how is this different from any other warning that is
considered pointless by the project?
Why not simply disable it?

> >> Would you be okay with the warning if it was only diagnosed when the
> >> source location of the semi-colon is not immediately preceded by a
> >> macro expansion location? e.g.,
> >>
> >> EMPTY_EXPANSION(12); // Not diagnosed
> >> EMPTY_EXPANSION; // Not diagnosed
> >> ; // Diagnosed
> > This could work provided that all empty space preceding the semicolon is 
> > ignored (as the macro could be separated by space(s) or newline(s).
> > I’m not sure the complexity would be worth it, as I haven’t seen bugs 
> > arising from extra semicolons and empty statements,
> > but it would take care of my use case.
> >
>
> Or rather when the *previous* semicolon is coming from a macro.
I don't like this. clang is already wa-a-ay too forgiving about
macros, it almost ignores almost every warning in macros.
It was an *intentional* decision to still warn on useless ';' after
non-empty macros.
So at most i would be ok with emitting the macro case under -W{flag}-macro.
Thoughts?

> >> ~Aaron
> >>
> >>> Regards,
> >>> George
Roman.

>  On Nov 20, 2018, at 10:59 AM, Roman Lebedev via cfe-commits 
>   wrote:
> 
>  Author: lebedevri
>  Date: Tue Nov 20 10:59:05 2018
>  New Revision: 347339
> 
>  URL: http://llvm.org/viewvc/llvm-project?rev=347339=rev
>  Log:
>  [clang][Parse] Diagnose useless null statements / empty init-statements
> 
>  Summary:
>  clang has `-Wextra-semi` (D43162), which is not dictated by the 
>  currently selected standard.
>  While that is great, there is at least one more source of need-less 
>  semis - 'null statements'.
>  Sometimes, they are needed:
>  ```
>  for(int x = 0; continueToDoWork(x); x++)
>  ; // Ugly code, but the semi is needed here.
>  ```
> 
>  But sometimes they are just there for no reason:
>  ```
>  switch(X) {
>  case 0:
>  return -2345;
>  case 5:
>  return 0;
>  default:
>  return 42;
>  }; // <- oops
> 
>  ;;; <- PS, still not diagnosed. Clearly this is junk.
>  ```
> 
>  Additionally:
>  ```
>  if(; // <- empty init-statement
>    true)
>  ;
> 
>  switch (; // empty init-statement
> x) {
>  ...
>  }
> 
>  for (; // <- empty init-statement
>  int y : S())
>  ;
>  }
> 
>  As usual, things may or may not go sideways in the presence of macros.
>  While evaluating this diag on my codebase of interest, it was 
>  unsurprisingly
>  discovered that Google Test macros are *very* prone to this.
>  And it seems many issues are deep within the GTest itself, not
>  in the snippets passed from the codebase that uses GTest.
> 
>  So after some thought, i decided not do issue a diagnostic if the semi
>  is within *any* macro, be it either from the normal header, or system 
>  header.
> 
>  Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=39111 | PR39111 ]]
> 
>  Reviewers: rsmith, aaron.ballman, efriedma
> 
>  Reviewed By: aaron.ballman
> 
>  Subscribers: cfe-commits
> 
>  Differential Revision: https://reviews.llvm.org/D52695
> 
>  Added:
> 
>  cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp
> cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp
> 

[PATCH] D55289: [analyzer] MoveChecker Pt.5: Improve invalidation policies.

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hi Artem,
The change looks fine, just some nits.




Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:537
+const MemRegion *ThisRegion = nullptr;
+if (const auto *IC = dyn_cast_or_null(Call))
+  ThisRegion = IC->getCXXThisVal().getAsRegion();

It looks like Call is already checked for null, and we can use just dyn_cast.



Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:545
+  if (ThisRegion != Region)
+if (std::find(Regions.begin(), Regions.end(), Region) != Regions.end())
+  State = removeFromState(State, Region);

NoQ wrote:
> This is clumsy. I think we shouldn't include non-invalidated regions in the 
> `ExplicitRegions` array in the first place.
Just a reminder: we have `llvm::find` and a bunch of nice related range 
wrappers.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55289



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

[PATCH] D21508: Diagnose friend function template redefinitions

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 176763.
sepavloff added a comment.

Updated patch

The fix for https://bugs.llvm.org/show_bug.cgi?id=39742 put a test
case, which is not a valid code. The error is detected with this
patch, tests are updated accordingly.


Repository:
  rC Clang

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

https://reviews.llvm.org/D21508

Files:
  include/clang/AST/DeclBase.h
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  test/Modules/friend-definition.cpp
  test/SemaCXX/friend-template-redecl.cpp
  test/SemaCXX/friend2.cpp

Index: test/SemaCXX/friend2.cpp
===
--- test/SemaCXX/friend2.cpp
+++ test/SemaCXX/friend2.cpp
@@ -129,6 +129,83 @@
 void func_22() {} // expected-error{{redefinition of 'func_22'}}
 
 
+// Case of template friend functions.
+
+template void func_31(T *x);
+template
+struct C31a {
+  template friend void func_31(T *x) {}
+};
+template
+struct C31b {
+  template friend void func_31(T *x) {}
+};
+
+
+template inline void func_32(T *x) {}
+template
+struct C32a {
+  template friend void func_32(T *x) {}
+};
+template
+struct C32b {
+  template friend void func_32(T *x) {}
+};
+
+
+template
+struct C33a {
+  template friend void func_33(T *x) {}
+};
+template
+struct C33b {
+  template friend void func_33(T *x) {}
+};
+
+
+template inline void func_34(T *x) {}  // expected-note{{previous definition is here}}
+template
+struct C34 {
+  template friend void func_34(T *x) {} // expected-error{{redefinition of 'func_34'}}
+};
+
+C34 v34;  // expected-note{{in instantiation of template class 'C34' requested here}}
+
+
+template inline void func_35(T *x);
+template
+struct C35a {
+  template friend void func_35(T *x) {} // expected-note{{previous definition is here}}
+};
+template
+struct C35b {
+  template friend void func_35(T *x) {} // expected-error{{redefinition of 'func_35'}}
+};
+
+C35a v35a;
+C35b v35b;  // expected-note{{in instantiation of template class 'C35b' requested here}}
+
+
+template void func_36(T *x);
+template
+struct C36 {
+  template friend void func_36(T *x) {}  // expected-error{{redefinition of 'func_36'}}
+ // expected-note@-1{{previous definition is here}}
+};
+
+C36 v36a;
+C36 v36b;  //expected-note{{in instantiation of template class 'C36' requested here}}
+
+
+template void func_37(T *x);
+template
+struct C37 {
+  template friend void func_37(T *x) {} // expected-note{{previous definition is here}}
+};
+
+C37 v37;
+template void func_37(T *x) {} // expected-error{{redefinition of 'func_37'}}
+
 
 namespace pr22307 {
 
@@ -235,3 +312,15 @@
   cache.insert();
 }
 }
+
+namespace PR39742 {
+template
+struct wrapper {
+  template
+  friend void friend_function_template() {}  // expected-error{{redefinition of 'friend_function_template'}}
+ // expected-note@-1{{previous definition is here}}
+};
+
+wrapper x;
+wrapper y;  // expected-note{{in instantiation of template class 'PR39742::wrapper' requested here}}
+}
Index: test/SemaCXX/friend-template-redecl.cpp
===
--- test/SemaCXX/friend-template-redecl.cpp
+++ test/SemaCXX/friend-template-redecl.cpp
@@ -18,14 +18,3 @@
   foo(x);
   bar(x);
 }
-
-namespace PR39742 {
-template
-struct wrapper {
-  template
-  friend void friend_function_template() {}
-};
-
-wrapper x;
-wrapper y;
-}
Index: test/Modules/friend-definition.cpp
===
--- test/Modules/friend-definition.cpp
+++ test/Modules/friend-definition.cpp
@@ -7,6 +7,7 @@
 #pragma clang module begin A
 template struct A {
   friend A operator+(const A&, const A&) { return {}; }
+  template friend void func_1(const A&, const T2 &) {}
 };
 #pragma clang module end
 #pragma clang module endbuild
@@ -36,4 +37,5 @@
 void h() {
   A a;
   a + a;
+  func_1(a, 0);
 }
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1817,7 +1817,9 @@
   // If the original function was part of a friend declaration,
   // inherit its namespace state and add it to the owner.
   if (isFriend) {
-PrincipalDecl->setObjectOfFriendDecl();
+Function->setObjectOfFriendDecl();
+if (FunctionTemplateDecl *FT = Function->getDescribedFunctionTemplate())
+  FT->setObjectOfFriendDecl();
 DC->makeDeclVisibleInContext(PrincipalDecl);
 
 bool QueuedInstantiation = false;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -12744,6 +12744,29 @@
   }
 }
   }
+
+  if (!Definition)
+// Similar to friend functions a friend function template may be a
+// definition and do not have a 

[PATCH] D55262: [OpenCL] Fix for TBAA information of pointer after addresspacecast

[John McCall via Phabricator via cfe-commits]

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

LGTM.  Although this also fixes bugs with unaligned pointers that you might 
consider adding tests for.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55262



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

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

[John McCall via Phabricator via cfe-commits]

rjmccall added a subscriber: akyrtzi.
rjmccall added a comment.

I have a lot of comments about various things that need to be fixed, but I just 
want to take a moment to say that this is a tremendously impressive patch: 
given only some very high-level directions, you've done a great job figuring 
out what you needed to do to carry that out and make it work.  Really well done.




Comment at: lib/AST/ASTDumper.cpp:346
+break;
+  }
   switch (EPI.RefQualifier) {

We must have some existing code to print an address space.



Comment at: lib/AST/ItaniumMangle.cpp:1507
+Qualifiers MethodQuals = Qualifiers::fromCVRUMask(
+Method->getTypeQualifiers().getCVRUQualifiers());
 // We do not consider restrict a distinguishing attribute for overloading

You can overload based on the address space, right?  I think it needs to be 
mangled.



Comment at: lib/AST/TypePrinter.cpp:804
 
-  if (unsigned quals = T->getTypeQuals()) {
+  if (unsigned quals = T->getTypeQuals().getCVRUQualifiers()) {
 OS << ' ';

You should be printing all the qualifiers here, including address spaces.  If 
you want to suppress an implicit OpenCL address-space qualifier, that seems 
reasonable.



Comment at: lib/CodeGen/CGDebugInfo.cpp:2593
   getOrCreateInstanceMethodType(CGM.getContext().getPointerType(QualType(
-Ty->getClass(), FPT->getTypeQuals())),
+Ty->getClass(), 
FPT->getTypeQuals().getCVRUQualifiers())),
 FPT, U),

I think this is just `GetThisType` again.  Maybe there should be a version of 
that takes a record and an FPT; you could make it a static method on 
`CXXMethodDecl`.  At any rate, I don't think you shouldn't be dropping the 
address space.



Comment at: lib/Index/USRGeneration.cpp:274
+if (unsigned quals = MD->getTypeQualifiers().getCVRUQualifiers())
   Out << (char)('0' + quals);
 switch (MD->getRefQualifier()) {

Paging @akyrtzi here.  The address-space qualifier should be part of the USR 
but I don't think if there's a defined schema for that.



Comment at: lib/Parse/ParseCXXInlineMethods.cpp:419
 Sema::CXXThisScopeRAII ThisScope(Actions, Method->getParent(),
- Method->getTypeQualifiers(),
+ 
Method->getTypeQualifiers().getCVRUQualifiers(),
  getLangOpts().CPlusPlus11);

This should take the full qualifiers.  You can see how it ends up getting 
reassembled into the `this` type in the `CXXThisScopeRAII` ctor, and that type 
definitely needs to be address-space-qualified.



Comment at: lib/Sema/SemaCodeComplete.cpp:1046
+Qualifiers MethodQuals = Qualifiers::fromCVRMask(
+Method->getTypeQualifiers().getCVRQualifiers());
 if (ObjectTypeQualifiers == MethodQuals)

Hmm.  This is actually going to break code completion for these methods, I 
think: IIUC in OpenCL every pointer and l-value is address-space-qualified, so 
only considering the CVRU qualifiers means that there will always be "extra" 
qualifiers on the pointer and code completion will think it doesn't match.

...I know code completion isn't your top priority, but this is probably easy 
enough to fix.



Comment at: lib/Sema/SemaCodeComplete.cpp:3701
+  Results.setObjectTypeQualifiers(Qualifiers::fromCVRMask(
+  CurMethod->getTypeQualifiers().getCVRQualifiers()));
   MemberCompletionRecord = CurMethod->getParent();

This seems to be intentionally dropping non-CVR qualifiers, and I don't know 
why you ever would want to do that.



Comment at: lib/Sema/SemaDeclCXX.cpp:11963
+BaseType,
+CopyAssignOperator->getTypeQualifiers().getCVRUQualifiers()),
+VK_LValue, BasePath);

This should definitely not be dropping address spaces.  You'll need to make a 
second patch to handle special members correctly, so you don't need to add a 
test for this yet, but you might as well get the logic right right now.



Comment at: lib/Sema/SemaDeclCXX.cpp:12332
+BaseType,
+MoveAssignOperator->getTypeQualifiers().getCVRQualifiers()),
+VK_LValue, BasePath);

Same thing as above.



Comment at: lib/Sema/SemaOverload.cpp:1146
+unsigned OldQuals = OldMethod->getTypeQualifiers().getCVRUQualifiers();
+unsigned NewQuals = NewMethod->getTypeQualifiers().getCVRUQualifiers();
 if (!getLangOpts().CPlusPlus14 && NewMethod->isConstexpr() &&

This is an algorithm that I think you need to get right and which shouldn't 
drop address spaces.



Comment 

[PATCH] D54872: [clangd] Add ability to not use -resource-dir at all

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added a comment.

It doesn't seem like there is any difference in how -resource-dir and /imsvc 
are handled: they are all added as -internal-isystem

In MSVCToolChain::AddClangSystemIncludeArgs (MSCV.cpp):

  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, getDriver().ResourceDir,
  "include");
  }
  
  // Add %INCLUDE%-like directories from the -imsvc flag.
  for (const auto  : DriverArgs.getAllArgValues(options::OPT__SLASH_imsvc))
addSystemInclude(DriverArgs, CC1Args, Path);

which in the example above would result in:

  -cc1  -internal-isystem 
"D:\\git\\llvm\\build-vs2017\\RelWithDebInfo\\bin\\..\\lib\\clang\\8.0.0\\include"
 -internal-isystem "C:\\Program Files (x86)\\Microsoft Visual 
Studio\\2017\\Professional\\VC\\Tools\\MSVC\\14.15.26726\\include" 
-internal-isystem "C:\\Program Files (x86)\\Microsoft Visual 
Studio\\2017\\Professional\\VC\\Tools\\MSVC\\14.15.26726\\atlmfc\\include"




Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54872



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

[PATCH] D51832: [clang-tidy/checks] Update objc-property-declaration check to allow arbitrary acronyms and initialisms 

[Stephane Moore via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348331: [clang-tidy/checks] Update objc-property-declaration 
check to allow arbitrary… (authored by stephanemoore, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51832?vs=176756=176759#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D51832

Files:
  clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst
  clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-additional.m
  clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-custom.m
  clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m

Index: clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
===
--- clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
+++ clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
@@ -1,8 +1,12 @@
 // RUN: %check_clang_tidy %s objc-property-declaration %t
+@class CIColor;
+@class NSArray;
 @class NSData;
 @class NSString;
 @class UIViewController;
 
+typedef void *CGColorRef;
+
 @interface Foo
 @property(assign, nonatomic) int NotCamelCase;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
@@ -23,6 +27,9 @@
 @property(assign, nonatomic) int enableGLAcceleration;
 @property(assign, nonatomic) int ID;
 @property(assign, nonatomic) int hasADog;
+@property(nonatomic, readonly) CGColorRef CGColor;
+@property(nonatomic, readonly) CIColor *CIColor;
+@property(nonatomic, copy) NSArray *IDs;
 @end
 
 @interface Foo (Bar)
Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -29,104 +29,12 @@
 // For CategoryProperty especially in categories of system class,
 // to avoid naming conflict, the suggested naming style is
 // 'abc_lowerCamelCase' (adding lowercase prefix followed by '_').
+// Regardless of the style, all acronyms and initialisms should be capitalized.
 enum NamingStyle {
   StandardProperty = 1,
   CategoryProperty = 2,
 };
 
-/// The acronyms are aggregated from multiple sources including
-/// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
-///
-/// Keep this list sorted.
-constexpr llvm::StringLiteral DefaultSpecialAcronyms[] = {
-"[2-9]G",
-"ACL",
-"API",
-"APN",
-"APNS",
-"AR",
-"ARGB",
-"ASCII",
-"AV",
-"BGRA",
-"CA",
-"CDN",
-"CF",
-"CG",
-"CI",
-"CRC",
-"CV",
-"CMYK",
-"DNS",
-"FPS",
-"FTP",
-"GIF",
-"GL",
-"GPS",
-"GUID",
-"HD",
-"HDR",
-"HMAC",
-"HTML",
-"HTTP",
-"HTTPS",
-"HUD",
-"ID",
-"JPG",
-"JS",
-"JSON",
-"LAN",
-"LZW",
-"LTR",
-"MAC",
-"MD",
-"MDNS",
-"MIDI",
-"NS",
-"OS",
-"P2P",
-"PDF",
-"PIN",
-"PNG",
-"POI",
-"PSTN",
-"PTR",
-"QA",
-"QOS",
-"RGB",
-"RGBA",
-"RGBX",
-"RIPEMD",
-"ROM",
-"RPC",
-"RTF",
-"RTL",
-"SC",
-"SDK",
-"SHA",
-"SQL",
-"SSO",
-"TCP",
-"TIFF",
-"TOS",
-"TTS",
-"UI",
-"URI",
-"URL",
-"UUID",
-"VC",
-"VO",
-"VOIP",
-"VPN",
-"VR",
-"W",
-"WAN",
-"X",
-"XML",
-"Y",
-"Z",
-};
-
 /// For now we will only fix 'CamelCase' or 'abc_CamelCase' property to
 /// 'camelCase' or 'abc_camelCase'. For other cases the users need to
 /// come up with a proper name by their own.
@@ -150,26 +58,26 @@
   return FixItHint();
 }
 
-std::string AcronymsGroupRegex(llvm::ArrayRef EscapedAcronyms) {
-  return "(" +
- llvm::join(EscapedAcronyms.begin(), EscapedAcronyms.end(), "s?|") +
- "s?)";
-}
-
-std::string validPropertyNameRegex(llvm::ArrayRef EscapedAcronyms,
-   bool UsedInMatcher) {
+std::string validPropertyNameRegex(bool UsedInMatcher) {
   // Allow any of these names:
   // foo
   // fooBar
   // url
   // urlString
+  // ID
+  // IDs
   // URL
   // URLString
   // bundleID
+  // CIColor
+  //
+  // Disallow names of this form:
+  // LongString
+  //
+  // aRbITRaRyCapS is allowed to avoid generating false positives for names
+  // like isVitaminBSupplement, CProgrammingLanguage, and isBeforeM.
   

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Hyrum Wright via Phabricator via cfe-commits]

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

Fix double space.


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

https://reviews.llvm.org/D55245

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationComparisonCheck.cpp
  clang-tidy/abseil/DurationRewriter.cpp
  clang-tidy/abseil/DurationRewriter.h
  clang-tidy/abseil/DurationSubtractionCheck.cpp
  clang-tidy/abseil/DurationSubtractionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-duration-subtraction.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-duration-subtraction.cpp

Index: test/clang-tidy/abseil-duration-subtraction.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-duration-subtraction.cpp
@@ -0,0 +1,112 @@
+// RUN: %check_clang_tidy %s abseil-duration-subtraction %t
+
+// Mimic the implementation of absl::Duration
+namespace absl {
+
+class Duration {};
+class Time{};
+
+Duration Nanoseconds(long long);
+Duration Microseconds(long long);
+Duration Milliseconds(long long);
+Duration Seconds(long long);
+Duration Minutes(long long);
+Duration Hours(long long);
+
+#define GENERATE_DURATION_FACTORY_OVERLOADS(NAME) \
+  Duration NAME(float n); \
+  Duration NAME(double n);\
+  template\
+  Duration NAME(T n);
+
+GENERATE_DURATION_FACTORY_OVERLOADS(Nanoseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Microseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Milliseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Seconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Minutes);
+GENERATE_DURATION_FACTORY_OVERLOADS(Hours);
+#undef GENERATE_DURATION_FACTORY_OVERLOADS
+
+using int64_t = long long int;
+
+double ToDoubleHours(Duration d);
+double ToDoubleMinutes(Duration d);
+double ToDoubleSeconds(Duration d);
+double ToDoubleMilliseconds(Duration d);
+double ToDoubleMicroseconds(Duration d);
+double ToDoubleNanoseconds(Duration d);
+int64_t ToInt64Hours(Duration d);
+int64_t ToInt64Minutes(Duration d);
+int64_t ToInt64Seconds(Duration d);
+int64_t ToInt64Milliseconds(Duration d);
+int64_t ToInt64Microseconds(Duration d);
+int64_t ToInt64Nanoseconds(Duration d);
+
+// Relational Operators
+constexpr bool operator<(Duration lhs, Duration rhs);
+constexpr bool operator>(Duration lhs, Duration rhs);
+constexpr bool operator>=(Duration lhs, Duration rhs);
+constexpr bool operator<=(Duration lhs, Duration rhs);
+constexpr bool operator==(Duration lhs, Duration rhs);
+constexpr bool operator!=(Duration lhs, Duration rhs);
+
+// Additive Operators
+inline Time operator+(Time lhs, Duration rhs);
+inline Time operator+(Duration lhs, Time rhs);
+inline Time operator-(Time lhs, Duration rhs);
+inline Duration operator-(Time lhs, Time rhs);
+
+}  // namespace absl
+
+void f() {
+  double x;
+  absl::Duration d, d1;
+
+  x = absl::ToDoubleSeconds(d) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(1))
+  x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(d1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - d1);
+  x = absl::ToDoubleSeconds(d) - 6.5 - 8.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(6.5)) - 8.0;
+  x = absl::ToDoubleHours(d) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleHours(d - absl::Hours(1))
+  x = absl::ToDoubleMinutes(d) - 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMinutes(d - absl::Minutes(1))
+  x = absl::ToDoubleMilliseconds(d) - 9;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMilliseconds(d - absl::Milliseconds(9))
+  x = absl::ToDoubleMicroseconds(d) - 9;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleMicroseconds(d - absl::Microseconds(9))
+  x = absl::ToDoubleNanoseconds(d) - 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+  // CHECK-FIXES: absl::ToDoubleNanoseconds(d - absl::Nanoseconds(42))
+
+  // We can rewrite the argument of the duration conversion
+#define THIRTY absl::Seconds(30)
+  x = absl::ToDoubleSeconds(THIRTY) - 1.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: 

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:1304
+RHSTy = ResultTy;
+  }
+

leonardchan wrote:
> rjmccall wrote:
> > leonardchan wrote:
> > > rjmccall wrote:
> > > > leonardchan wrote:
> > > > > rjmccall wrote:
> > > > > > ebevhan wrote:
> > > > > > > rjmccall wrote:
> > > > > > > > leonardchan wrote:
> > > > > > > > > rjmccall wrote:
> > > > > > > > > > ebevhan wrote:
> > > > > > > > > > > rjmccall wrote:
> > > > > > > > > > > > Hmm.  So adding a signed integer to an unsigned 
> > > > > > > > > > > > fixed-point type always converts the integer to 
> > > > > > > > > > > > unsigned?  That's a little weird, but only because the 
> > > > > > > > > > > > fixed-point rule seems to be the other way.  Anyway, I 
> > > > > > > > > > > > assume it's not a bug in the spec.
> > > > > > > > > > > `handleFixedPointConversion` only calculates the result 
> > > > > > > > > > > type of the expression, not the calculation type. The 
> > > > > > > > > > > final result type of the operation will be the unsigned 
> > > > > > > > > > > fixed-point type, but the calculation itself will be done 
> > > > > > > > > > > in a signed type with enough precision to represent both 
> > > > > > > > > > > the signed integer and the unsigned fixed-point type. 
> > > > > > > > > > > 
> > > > > > > > > > > Though, if the result ends up being negative, the final 
> > > > > > > > > > > result is undefined unless the type is saturating. I 
> > > > > > > > > > > don't think there is a test for the saturating case 
> > > > > > > > > > > (signed integer + unsigned saturating fixed-point) in the 
> > > > > > > > > > > SaturatedAddition tests.
> > > > > > > > > > > `handleFixedPointConversion` only calculates the result 
> > > > > > > > > > > type of the expression, not the calculation type.
> > > > > > > > > > 
> > > > > > > > > > Right, I understand that, but the result type of the 
> > > > > > > > > > expression obviously impacts the expressive range of the 
> > > > > > > > > > result, since you can end up with a negative value.
> > > > > > > > > > 
> > > > > > > > > > Hmm.  With that said, if the general principle is to 
> > > > > > > > > > perform the operation with full precision on the original 
> > > > > > > > > > operand values, why are unsigned fixed-point operands 
> > > > > > > > > > coerced to their corresponding signed types *before* the 
> > > > > > > > > > operation?  This is precision-limiting if the unsigned 
> > > > > > > > > > representation doesn't use a padding bit.  That seems like 
> > > > > > > > > > a bug in the spec.
> > > > > > > > > > Hmm. With that said, if the general principle is to perform 
> > > > > > > > > > the operation with full precision on the original operand 
> > > > > > > > > > values, why are unsigned fixed-point operands coerced to 
> > > > > > > > > > their corresponding signed types *before* the operation? 
> > > > > > > > > > This is precision-limiting if the unsigned representation 
> > > > > > > > > > doesn't use a padding bit. That seems like a bug in the 
> > > > > > > > > > spec.
> > > > > > > > > 
> > > > > > > > > Possibly. When the standard mentions converting from signed 
> > > > > > > > > to unsigned fixed point, the only requirement involved is 
> > > > > > > > > conservation of magnitude (the number of integral bits 
> > > > > > > > > excluding the sign)
> > > > > > > > > 
> > > > > > > > > ```
> > > > > > > > > when signed and unsigned fixed-point types are mixed, the 
> > > > > > > > > unsigned type is converted to the corresponding signed type, 
> > > > > > > > > and this should go without loss of magnitude
> > > > > > > > > ```
> > > > > > > > > 
> > > > > > > > > This is just speculation, but I'm under the impression that 
> > > > > > > > > not as much "attention" was given for unsigned types as for 
> > > > > > > > > signed types since "`In the embedded processor world, support 
> > > > > > > > > for unsigned fixed-point data types is rare; normally only 
> > > > > > > > > signed fixed-point data types are supported`", but was kept 
> > > > > > > > > anyway since unsigned types are used a lot.
> > > > > > > > > 
> > > > > > > > > ```
> > > > > > > > > However, to disallow unsigned fixed-point arithmetic from 
> > > > > > > > > programming languages (in general, and from C in particular) 
> > > > > > > > > based on this observation, seems overly restrictive.
> > > > > > > > > ```
> > > > > > > > > 
> > > > > > > > > I also imagine that if the programmer needs more precision, 
> > > > > > > > > the correct approach would be to cast up to a type with a 
> > > > > > > > > higher scale. The standard seems to make an effort to expose 
> > > > > > > > > as much in regards to the underlying fixed point types as 
> > > > > > > > > possible:
> > > > > > > > > 
> > > > > > > > > ```
> > > > > > > > > it should be possible to write fixed-point algorithms that 
> > > > > > > > > are independent of the actual fixed-point hardware 

[PATCH] D51832: [clang-tidy/checks] Update objc-property-declaration check to allow arbitrary acronyms and initialisms 

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 176756.
stephanemoore added a comment.

Rebased changes and resolved merge conflicts.
Moved release notes update next to other release notes regarding updates to 
existing checks.


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

https://reviews.llvm.org/D51832

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/objc-property-declaration.rst
  test/clang-tidy/objc-property-declaration-additional.m
  test/clang-tidy/objc-property-declaration-custom.m
  test/clang-tidy/objc-property-declaration.m

Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -1,8 +1,12 @@
 // RUN: %check_clang_tidy %s objc-property-declaration %t
+@class CIColor;
+@class NSArray;
 @class NSData;
 @class NSString;
 @class UIViewController;
 
+typedef void *CGColorRef;
+
 @interface Foo
 @property(assign, nonatomic) int NotCamelCase;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
@@ -23,6 +27,9 @@
 @property(assign, nonatomic) int enableGLAcceleration;
 @property(assign, nonatomic) int ID;
 @property(assign, nonatomic) int hasADog;
+@property(nonatomic, readonly) CGColorRef CGColor;
+@property(nonatomic, readonly) CIColor *CIColor;
+@property(nonatomic, copy) NSArray *IDs;
 @end
 
 @interface Foo (Bar)
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ /dev/null
@@ -1,18 +0,0 @@
-// RUN: %check_clang_tidy %s objc-property-declaration %t \
-// RUN: -config='{CheckOptions: \
-// RUN:  [{key: objc-property-declaration.Acronyms, value: "ABC;TGIF"}, \
-// RUN:   {key: objc-property-declaration.IncludeDefaultAcronyms, value: 0}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-// CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
-@property(assign, nonatomic) int ABCCustomPrefix;
-@property(strong, nonatomic) NSString *ABC_custom_prefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFIgnoreStandardAcronym;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(strong, nonatomic) NSString *TGIF;
-@end
Index: test/clang-tidy/objc-property-declaration-additional.m
===
--- test/clang-tidy/objc-property-declaration-additional.m
+++ /dev/null
@@ -1,15 +0,0 @@
-// RUN: %check_clang_tidy %s objc-property-declaration %t \
-// RUN: -config='{CheckOptions: \
-// RUN:  [{key: objc-property-declaration.Acronyms, value: "ABC;TGIF"}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-// CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
-@property(assign, nonatomic) int ABCCustomPrefix;
-@property(strong, nonatomic) NSString *ABC_custom_prefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFShouldIncludeStandardAcronym;
-@end
Index: docs/clang-tidy/checks/objc-property-declaration.rst
===
--- docs/clang-tidy/checks/objc-property-declaration.rst
+++ docs/clang-tidy/checks/objc-property-declaration.rst
@@ -47,23 +47,8 @@
 
 .. option:: Acronyms
 
-   Semicolon-separated list of custom acronyms that can be used as a prefix
-   or a suffix of property names.
-
-   By default, appends to the list of default acronyms (
-   ``IncludeDefaultAcronyms`` set to ``1``).
-   If ``IncludeDefaultAcronyms`` is set to ``0``, instead replaces the
-   default list of acronyms.
+   This option is deprecated and ignored.
 
 .. option:: IncludeDefaultAcronyms
 
-   Integer value 

Re: r348325 - [Sema] Remove some conditions of a failing assert

[Richard Smith via cfe-commits]

On Tue, 4 Dec 2018 at 16:46, Erik Pilkington via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: epilk
> Date: Tue Dec  4 16:43:11 2018
> New Revision: 348325
>
> URL: http://llvm.org/viewvc/llvm-project?rev=348325=rev
> Log:
> [Sema] Remove some conditions of a failing assert
>
> We should have been checking that this state is consistent, but its
> possible for it to be filled later, so it isn't really sound to check
> it here anyways.
>
> Fixes llvm.org/PR39742
>
> Modified:
> cfe/trunk/lib/AST/DeclTemplate.cpp
> cfe/trunk/test/SemaCXX/friend-template-redecl.cpp
>
> Modified: cfe/trunk/lib/AST/DeclTemplate.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclTemplate.cpp?rev=348325=348324=348325=diff
>
> ==
> --- cfe/trunk/lib/AST/DeclTemplate.cpp (original)
> +++ cfe/trunk/lib/AST/DeclTemplate.cpp Tue Dec  4 16:43:11 2018
> @@ -329,8 +329,6 @@ void FunctionTemplateDecl::mergePrevDecl
>
>// Ensure we don't leak any important state.
>assert(ThisCommon->Specializations.size() == 0 &&
> - !ThisCommon->InstantiatedFromMember.getPointer() &&
> - !ThisCommon->InstantiatedFromMember.getInt() &&
>   "Can't merge incompatible declarations!");
>
>Base::Common = PrevCommon;
>
> Modified: cfe/trunk/test/SemaCXX/friend-template-redecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/friend-template-redecl.cpp?rev=348325=348324=348325=diff
>
> ==
> --- cfe/trunk/test/SemaCXX/friend-template-redecl.cpp (original)
> +++ cfe/trunk/test/SemaCXX/friend-template-redecl.cpp Tue Dec  4 16:43:11
> 2018
> @@ -18,3 +18,14 @@ void f() {
>foo(x);
>bar(x);
>  }
> +
> +namespace PR39742 {
> +template
> +struct wrapper {
> +  template
> +  friend void friend_function_template() {}
> +};
> +
> +wrapper x;
> +wrapper y;
>

This is ill-formed due to the redefinition of the template; we should at
least have a FIXME to properly reject it.


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

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

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle updated this revision to Diff 176754.
malaperle added a comment.

Clang-format


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55250

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


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -616,7 +616,27 @@
 #define MACRO 2
 #undef macro
   )cpp",
-  "#define MACRO",
+  "```C++\n#define MACRO 1\n```",
+  },
+  {
+  R"cpp(// Macro
+#define MACRO 0
+#define MACRO2 ^MACRO
+  )cpp",
+  "```C++\n#define MACRO 0\n```",
+  },
+  {
+  R"cpp(// Macro
+#define MACRO {\
+  return 0;\
+}
+int main() ^MACRO
+  )cpp",
+  R"cpp(```C++
+#define MACRO {\
+  return 0;\
+}
+```)cpp",
   },
   {
   R"cpp(// Forward class declaration
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -565,12 +565,28 @@
 }
 
 /// Generate a \p Hover object given the macro \p MacroInf.
-static Hover getHoverContents(StringRef MacroName) {
-  Hover H;
-
-  H.contents.value = "#define ";
-  H.contents.value += MacroName;
+static Hover getHoverContents(MacroDecl Decl, ASTContext ) {
+  SourceManager  = ASTCtx.getSourceManager();
+  StringRef Definition = Decl.Name;
+
+  // Try to get the full definition, not just the name
+  SourceLocation StartLoc = Decl.Info->getDefinitionLoc();
+  SourceLocation EndLoc = Decl.Info->getDefinitionEndLoc();
+  if (EndLoc.isValid()) {
+EndLoc = Lexer::getLocForEndOfToken(EndLoc, 0, SM, ASTCtx.getLangOpts());
+bool Invalid;
+StringRef Buffer = SM.getBufferData(SM.getFileID(StartLoc), );
+if (!Invalid) {
+  unsigned StartOffset = SM.getFileOffset(StartLoc);
+  unsigned EndOffset = SM.getFileOffset(EndLoc);
+  if (EndOffset <= Buffer.size() && StartOffset < EndOffset)
+Definition = Buffer.substr(StartOffset, EndOffset - StartOffset);
+}
+  }
 
+  Hover H;
+  H.contents.kind = MarkupKind::Markdown;
+  H.contents.value = formatv("```C++\n#define {0}\n```", Definition);
   return H;
 }
 
@@ -694,7 +710,7 @@
   auto Symbols = getSymbolAtPosition(AST, SourceLocationBeg);
 
   if (!Symbols.Macros.empty())
-return getHoverContents(Symbols.Macros[0].Name);
+return getHoverContents(Symbols.Macros[0], AST.getASTContext());
 
   if (!Symbols.Decls.empty())
 return getHoverContents(Symbols.Decls[0].D);


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -616,7 +616,27 @@
 #define MACRO 2
 #undef macro
   )cpp",
-  "#define MACRO",
+  "```C++\n#define MACRO 1\n```",
+  },
+  {
+  R"cpp(// Macro
+#define MACRO 0
+#define MACRO2 ^MACRO
+  )cpp",
+  "```C++\n#define MACRO 0\n```",
+  },
+  {
+  R"cpp(// Macro
+#define MACRO {\
+  return 0;\
+}
+int main() ^MACRO
+  )cpp",
+  R"cpp(```C++
+#define MACRO {\
+  return 0;\
+}
+```)cpp",
   },
   {
   R"cpp(// Forward class declaration
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -565,12 +565,28 @@
 }
 
 /// Generate a \p Hover object given the macro \p MacroInf.
-static Hover getHoverContents(StringRef MacroName) {
-  Hover H;
-
-  H.contents.value = "#define ";
-  H.contents.value += MacroName;
+static Hover getHoverContents(MacroDecl Decl, ASTContext ) {
+  SourceManager  = ASTCtx.getSourceManager();
+  StringRef Definition = Decl.Name;
+
+  // Try to get the full definition, not just the name
+  SourceLocation StartLoc = Decl.Info->getDefinitionLoc();
+  SourceLocation EndLoc = Decl.Info->getDefinitionEndLoc();
+  if (EndLoc.isValid()) {
+EndLoc = Lexer::getLocForEndOfToken(EndLoc, 0, SM, ASTCtx.getLangOpts());
+bool Invalid;
+StringRef Buffer = SM.getBufferData(SM.getFileID(StartLoc), );
+if (!Invalid) {
+  unsigned StartOffset = SM.getFileOffset(StartLoc);
+  unsigned EndOffset = SM.getFileOffset(EndLoc);
+  if (EndOffset <= Buffer.size() && StartOffset < EndOffset)
+Definition = Buffer.substr(StartOffset, EndOffset - StartOffset);
+}
+  }
 
+  Hover H;
+  H.contents.kind = MarkupKind::Markdown;
+  H.contents.value = formatv("```C++\n#define {0}\n```", Definition);
   return H;
 }
 
@@ -694,7 +710,7 @@
   auto Symbols = getSymbolAtPosition(AST, SourceLocationBeg);
 
   if 

[PATCH] D51832: [clang-tidy/checks] Update objc-property-declaration check to allow arbitrary acronyms and initialisms 

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Chatted with Ben offline and he's okay with proceeding.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D51832



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

[clang-tools-extra] r348328 - [clang-query] Continue if compilation command not found for some files

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Tue Dec  4 18:02:40 2018
New Revision: 348328

URL: http://llvm.org/viewvc/llvm-project?rev=348328=rev
Log:
[clang-query] Continue if compilation command not found for some files

When searching for a code pattern in an entire project with a
compilation database it's tempting to run

```
clang-query **.cpp
```

And yet, that often breaks because some files are just not in the
compilation database: tests, sample code, etc..
clang-query should not stop when encountering such cases.

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

Modified:
clang-tools-extra/trunk/clang-query/tool/ClangQuery.cpp

Modified: clang-tools-extra/trunk/clang-query/tool/ClangQuery.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-query/tool/ClangQuery.cpp?rev=348328=348327=348328=diff
==
--- clang-tools-extra/trunk/clang-query/tool/ClangQuery.cpp (original)
+++ clang-tools-extra/trunk/clang-query/tool/ClangQuery.cpp Tue Dec  4 18:02:40 
2018
@@ -100,8 +100,19 @@ int main(int argc, const char **argv) {
   ClangTool Tool(OptionsParser.getCompilations(),
  OptionsParser.getSourcePathList());
   std::vector> ASTs;
-  if (Tool.buildASTs(ASTs) != 0)
+  int Status = Tool.buildASTs(ASTs);
+  int ASTStatus = 0;
+  if (Status == 1) {
+// Building ASTs failed.
 return 1;
+  } else if (Status == 2) {
+ASTStatus |= 1;
+llvm::errs() << "Failed to build AST for some of the files, "
+ << "results may be incomplete."
+ << "\n";
+  } else {
+assert(Status == 0 && "Unexpected status returned");
+  }
 
   QuerySession QS(ASTs);
 
@@ -134,5 +145,5 @@ int main(int argc, const char **argv) {
 }
   }
 
-  return 0;
+  return ASTStatus;
 }


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

r348327 - [asan] Add clang flag -fsanitize-address-use-odr-indicator

[Vitaly Buka via cfe-commits]

Author: vitalybuka
Date: Tue Dec  4 17:44:31 2018
New Revision: 348327

URL: http://llvm.org/viewvc/llvm-project?rev=348327=rev
Log:
[asan] Add clang flag -fsanitize-address-use-odr-indicator

Reviewers: eugenis, m.ostapenko, ygribov

Subscribers: hiraditya, llvm-commits

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

Added:
cfe/trunk/test/CodeGen/asan-globals-odr.cpp
Modified:
cfe/trunk/docs/ClangCommandLineReference.rst
cfe/trunk/docs/UsersManual.rst
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/SanitizerArgs.h
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/Driver/fsanitize.c

Modified: cfe/trunk/docs/ClangCommandLineReference.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangCommandLineReference.rst?rev=348327=348326=348327=diff
==
--- cfe/trunk/docs/ClangCommandLineReference.rst (original)
+++ cfe/trunk/docs/ClangCommandLineReference.rst Tue Dec  4 17:44:31 2018
@@ -800,6 +800,10 @@ Level of field padding for AddressSaniti
 
 Enable linker dead stripping of globals in AddressSanitizer
 
+.. option:: -fsanitize-address-use-odr-indicator, 
-fno-sanitize-address-use-odr-indicator
+
+Enable ODR indicator globals to avoid false ODR violation reports in partially 
sanitized programs at the cost of an increase in binary size
+
 .. option:: -fsanitize-address-poison-custom-array-cookie, 
-fno-sanitize-address-poison-custom-array-cookie
 
 Enable "poisoning" array cookies when allocating arrays with a custom operator 
new\[\] in Address Sanitizer, preventing accesses to the cookies from user 
code. An array cookie is a small implementation-defined header added to certain 
array allocations to record metadata such as the length of the array. Accesses 
to array cookies from user code are technically allowed by the standard but are 
more likely to be the result of an out-of-bounds array access.

Modified: cfe/trunk/docs/UsersManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=348327=348326=348327=diff
==
--- cfe/trunk/docs/UsersManual.rst (original)
+++ cfe/trunk/docs/UsersManual.rst Tue Dec  4 17:44:31 2018
@@ -3089,6 +3089,8 @@ Execute ``clang-cl /?`` to see a list of
   Level of field padding for AddressSanitizer
   -fsanitize-address-globals-dead-stripping
   Enable linker dead stripping of globals in 
AddressSanitizer
+  -fsanitize-address-use-odr-indicator
+  Enable ODR indicator globals to avoid false ODR 
violation reports in partially sanitized programs at the cost of an increase in 
binary size
   -fsanitize-address-poison-custom-array-cookie
   Enable poisoning array cookies when using custom 
operator new[] in AddressSanitizer
   -fsanitize-address-use-after-scope

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=348327=348326=348327=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Dec  4 17:44:31 2018
@@ -984,6 +984,14 @@ def fno_sanitize_address_poison_custom_a
 def fsanitize_address_globals_dead_stripping : Flag<["-"], 
"fsanitize-address-globals-dead-stripping">,
 Group,
 HelpText<"Enable linker dead stripping 
of globals in AddressSanitizer">;
+def fsanitize_address_use_odr_indicator
+: Flag<["-"], "fsanitize-address-use-odr-indicator">,
+  Group,
+  HelpText<"Enable ODR indicator globals to avoid false ODR violation 
reports in partially sanitized programs at the cost of an increase in binary 
size">;
+def fno_sanitize_address_use_odr_indicator
+: Flag<["-"], "fno-sanitize-address-use-odr-indicator">,
+  Group,
+  HelpText<"Disable ODR indicator globals">;
 def fsanitize_recover : Flag<["-"], "fsanitize-recover">, Group;
 def fno_sanitize_recover : Flag<["-"], "fno-sanitize-recover">,
Flags<[CoreOption, DriverOption]>,

Modified: cfe/trunk/include/clang/Driver/SanitizerArgs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/SanitizerArgs.h?rev=348327=348326=348327=diff
==
--- cfe/trunk/include/clang/Driver/SanitizerArgs.h (original)
+++ cfe/trunk/include/clang/Driver/SanitizerArgs.h Tue Dec  4 17:44:31 2018
@@ -38,6 +38,7 @@ class SanitizerArgs {
   bool AsanUseAfterScope = true;
   bool 

[PATCH] D54923: [Modules] Remove non-determinism while serializing DECL_CONTEXT_LEXICAL and DECL_RECORD

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Ping!


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

https://reviews.llvm.org/D54923



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

[PATCH] D55307: [analyzer] MoveChecker Pt.6: Suppress the warning for the few move-safe STL classes.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, george.karpenkov, szepet, 
rnkovacs, Szelethus.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, baloghadamsoftware.

List of such classes shamelessly copied from 
https://wiki.sei.cmu.edu/confluence/x/O3s-BQ - thanks @aaron.ballman!

Warnings for locals are not suppressed.

Had to pass the checker into the visitor and make some methods non-static 
because globals with constructors are discouraged.

Also converted use-after-move tests to use the system header simulator so that 
not to grow STL code duplication.


Repository:
  rC Clang

https://reviews.llvm.org/D55307

Files:
  lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/use-after-move.cpp

Index: test/Analysis/use-after-move.cpp
===
--- test/Analysis/use-after-move.cpp
+++ test/Analysis/use-after-move.cpp
@@ -13,47 +13,7 @@
 // RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
 // RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE
 
-namespace std {
-
-typedef __typeof(sizeof(int)) size_t;
-
-template 
-struct remove_reference;
-
-template 
-struct remove_reference { typedef _Tp type; };
-
-template 
-struct remove_reference<_Tp &> { typedef _Tp type; };
-
-template 
-struct remove_reference<_Tp &&> { typedef _Tp type; };
-
-template 
-typename remove_reference<_Tp>::type &(_Tp &&__t) {
-  return static_cast::type &&>(__t);
-}
-
-template 
-_Tp &(typename remove_reference<_Tp>::type &__t) noexcept {
-  return static_cast<_Tp &&>(__t);
-}
-
-template 
-void swap(T , T ) {
-  T c(std::move(a));
-  a = std::move(b);
-  b = std::move(c);
-}
-
-template 
-class vector {
-public:
-  vector();
-  void push_back(const T );
-};
-
-} // namespace std
+#include "Inputs/system-header-simulator-cxx.h"
 
 class B {
 public:
@@ -832,13 +792,26 @@
 
 class HasSTLField {
   std::vector V;
-  void foo() {
+  void testVector() {
 // Warn even in non-aggressive mode when it comes to STL, because
 // in STL the object is left in "valid but unspecified state" after move.
 std::vector W = std::move(V); // expected-note{{Object 'V' of type 'std::vector' is left in a valid but unspecified state after move}}
 V.push_back(123); // expected-warning{{Method called on moved-from object 'V'}}
   // expected-note@-1{{Method called on moved-from object 'V'}}
   }
+
+  std::unique_ptr P;
+  void testUniquePtr() {
+// unique_ptr remains in a well-defined state after move.
+std::unique_ptr Q = std::move(P);
+P.get();
+#ifdef AGGRESSIVE
+// expected-warning@-2{{Method called on moved-from object 'P'}}
+// expected-note@-4{{Object 'P' is moved}}
+// expected-note@-4{{Method called on moved-from object 'P'}}
+#endif
+*P += 1; // FIXME: Should warn that the pointer is null.
+  }
 };
 
 void localRValueMove(A &) {
@@ -846,3 +819,11 @@
   a.foo(); // expected-warning{{Method called on moved-from object 'a'}}
// expected-note@-1{{Method called on moved-from object 'a'}}
 }
+
+void localUniquePtr(std::unique_ptr P) {
+  // Even though unique_ptr is safe to use after move,
+  // reusing a local variable this way usually indicates a bug.
+  std::unique_ptr Q = std::move(P); // expected-note{{Object 'P' is moved}}
+  P.get(); // expected-warning{{Method called on moved-from object 'P'}}
+   // expected-note@-1{{Method called on moved-from object 'P'}}
+}
Index: test/Analysis/diagnostics/explicit-suppression.cpp
===
--- test/Analysis/diagnostics/explicit-suppression.cpp
+++ test/Analysis/diagnostics/explicit-suppression.cpp
@@ -19,6 +19,6 @@
 void testCopyNull(C *I, C *E) {
   std::copy(I, E, (C *)0);
 #ifndef SUPPRESSED
-  // expected-warning@../Inputs/system-header-simulator-cxx.h:670 {{Called C++ object pointer is null}}
+  // expected-warning@../Inputs/system-header-simulator-cxx.h:677 {{Called C++ object pointer is null}}
 #endif
 }
Index: test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- test/Analysis/Inputs/system-header-simulator-cxx.h
+++ test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -237,6 +237,13 @@
 return static_cast(a);
   }
 
+  template 
+  void swap(T , T ) {
+T c(std::move(a));
+a = std::move(b);
+b = std::move(c);
+  }
+
   template
   class vector {
 typedef T value_type;
@@ -770,6 +777,22 @@
 
 }
 
+#if __cplusplus >= 201103L
+namespace std {
+  template // TODO: Implement the stub for deleter.
+  class unique_ptr {
+  public:
+unique_ptr(const unique_ptr &) = delete;
+unique_ptr(unique_ptr &&);
+
+T *get() const;
+
+typename std::add_lvalue_reference::type operator*() const;
+T 

[PATCH] D55280: [CTU] Remove redundant error check

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

Szelethus added a comment.

From what I can see, this patch LGTM, but I lack the experience in the CTU 
department just yet to give any meaningful feedback.




Comment at: lib/CrossTU/CrossTranslationUnit.cpp:147
 
 llvm::Expected
 CrossTranslationUnitContext::getCrossTUDefinition(const FunctionDecl *FD,

Would it be worth to add a comment that this function never returns with 
`nullptr` on success?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55280



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

Re: [cfe-dev] r347339 - [clang][Parse] Diagnose useless null statements / empty init-statements

[Artem Dergachev via cfe-commits]

On 12/4/18 5:04 PM, George Karpenkov via cfe-dev wrote:



On Dec 4, 2018, at 4:47 PM, Aaron Ballman  wrote:

On Tue, Dec 4, 2018 at 7:35 PM George Karpenkov  wrote:

Hi Roman,

I’m against this new warning.

A very common idiom is defining a “DEBUG” macro to be a no-op in release, and a 
logging function otherwise.
The new introduced warning warns on all such cases (e.g. 'DEBUG(“mystring”);')

Yeah, that's not good.


As noted in the review, Clang warnings should generally not be used to diagnose 
style issues — and an extra semicolon does not seem to be a common bug source.

It is a problem in practice for us since we have projects which enable all 
available warnings and also enable “-Werror”.

Would you be okay with the warning if it was only diagnosed when the
source location of the semi-colon is not immediately preceded by a
macro expansion location? e.g.,

EMPTY_EXPANSION(12); // Not diagnosed
EMPTY_EXPANSION; // Not diagnosed
; // Diagnosed

This could work provided that all empty space preceding the semicolon is 
ignored (as the macro could be separated by space(s) or newline(s).
I’m not sure the complexity would be worth it, as I haven’t seen bugs arising 
from extra semicolons and empty statements,
but it would take care of my use case.



Or rather when the *previous* semicolon is coming from a macro.


~Aaron


Regards,
George



On Nov 20, 2018, at 10:59 AM, Roman Lebedev via cfe-commits 
 wrote:

Author: lebedevri
Date: Tue Nov 20 10:59:05 2018
New Revision: 347339

URL: http://llvm.org/viewvc/llvm-project?rev=347339=rev
Log:
[clang][Parse] Diagnose useless null statements / empty init-statements

Summary:
clang has `-Wextra-semi` (D43162), which is not dictated by the currently 
selected standard.
While that is great, there is at least one more source of need-less semis - 
'null statements'.
Sometimes, they are needed:
```
for(int x = 0; continueToDoWork(x); x++)
; // Ugly code, but the semi is needed here.
```

But sometimes they are just there for no reason:
```
switch(X) {
case 0:
return -2345;
case 5:
return 0;
default:
return 42;
}; // <- oops

;;; <- PS, still not diagnosed. Clearly this is junk.
```

Additionally:
```
if(; // <- empty init-statement
  true)
;

switch (; // empty init-statement
   x) {
...
}

for (; // <- empty init-statement
int y : S())
;
}

As usual, things may or may not go sideways in the presence of macros.
While evaluating this diag on my codebase of interest, it was unsurprisingly
discovered that Google Test macros are *very* prone to this.
And it seems many issues are deep within the GTest itself, not
in the snippets passed from the codebase that uses GTest.

So after some thought, i decided not do issue a diagnostic if the semi
is within *any* macro, be it either from the normal header, or system header.

Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=39111 | PR39111 ]]

Reviewers: rsmith, aaron.ballman, efriedma

Reviewed By: aaron.ballman

Subscribers: cfe-commits

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

Added:
   cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp
   cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp
Modified:
   cfe/trunk/docs/ReleaseNotes.rst
   cfe/trunk/include/clang/Basic/DiagnosticGroups.td
   cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
   cfe/trunk/include/clang/Parse/Parser.h
   cfe/trunk/lib/Parse/ParseExprCXX.cpp
   cfe/trunk/lib/Parse/ParseStmt.cpp

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=347339=347338=347339=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Tue Nov 20 10:59:05 2018
@@ -55,6 +55,63 @@ Major New Features
Improvements to Clang's diagnostics
^^^

+- ``-Wextra-semi-stmt`` is a new diagnostic that diagnoses extra semicolons,
+  much like ``-Wextra-semi``. This new diagnostic diagnoses all *unnecessary*
+  null statements (expression statements without an expression), unless: the
+  semicolon directly follows a macro that was expanded to nothing or if the
+  semicolon is within the macro itself. This applies to macros defined in 
system
+  headers as well as user-defined macros.
+
+  .. code-block:: c++
+
+  #define MACRO(x) int x;
+  #define NULLMACRO(varname)
+
+  void test() {
+; // <- warning: ';' with no preceding expression is a null statement
+
+while (true)
+  ; // OK, it is needed.
+
+switch (my_enum) {
+case E1:
+  // stuff
+  break;
+case E2:
+  ; // OK, it is needed.
+}
+
+MACRO(v0;) // Extra semicolon, but within macro, so ignored.
+
+MACRO(v1); // <- warning: ';' with no preceding expression is a null 
statement
+
+NULLMACRO(v2); // ignored, NULLMACRO expanded to nothing.
+  }
+

Re: r347339 - [clang][Parse] Diagnose useless null statements / empty init-statements

[George Karpenkov via cfe-commits]

> On Dec 4, 2018, at 4:47 PM, Aaron Ballman  wrote:
> 
> On Tue, Dec 4, 2018 at 7:35 PM George Karpenkov  wrote:
>> 
>> Hi Roman,
>> 
>> I’m against this new warning.
>> 
>> A very common idiom is defining a “DEBUG” macro to be a no-op in release, 
>> and a logging function otherwise.
>> The new introduced warning warns on all such cases (e.g. 
>> 'DEBUG(“mystring”);')
> 
> Yeah, that's not good.
> 
>> As noted in the review, Clang warnings should generally not be used to 
>> diagnose style issues — and an extra semicolon does not seem to be a common 
>> bug source.
>> 
>> It is a problem in practice for us since we have projects which enable all 
>> available warnings and also enable “-Werror”.
> 
> Would you be okay with the warning if it was only diagnosed when the
> source location of the semi-colon is not immediately preceded by a
> macro expansion location? e.g.,
> 
> EMPTY_EXPANSION(12); // Not diagnosed
> EMPTY_EXPANSION; // Not diagnosed
> ; // Diagnosed

This could work provided that all empty space preceding the semicolon is 
ignored (as the macro could be separated by space(s) or newline(s).
I’m not sure the complexity would be worth it, as I haven’t seen bugs arising 
from extra semicolons and empty statements,
but it would take care of my use case.

> 
> ~Aaron
> 
>> 
>> Regards,
>> George
>> 
>> 
>>> On Nov 20, 2018, at 10:59 AM, Roman Lebedev via cfe-commits 
>>>  wrote:
>>> 
>>> Author: lebedevri
>>> Date: Tue Nov 20 10:59:05 2018
>>> New Revision: 347339
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=347339=rev
>>> Log:
>>> [clang][Parse] Diagnose useless null statements / empty init-statements
>>> 
>>> Summary:
>>> clang has `-Wextra-semi` (D43162), which is not dictated by the currently 
>>> selected standard.
>>> While that is great, there is at least one more source of need-less semis - 
>>> 'null statements'.
>>> Sometimes, they are needed:
>>> ```
>>> for(int x = 0; continueToDoWork(x); x++)
>>> ; // Ugly code, but the semi is needed here.
>>> ```
>>> 
>>> But sometimes they are just there for no reason:
>>> ```
>>> switch(X) {
>>> case 0:
>>> return -2345;
>>> case 5:
>>> return 0;
>>> default:
>>> return 42;
>>> }; // <- oops
>>> 
>>> ;;; <- PS, still not diagnosed. Clearly this is junk.
>>> ```
>>> 
>>> Additionally:
>>> ```
>>> if(; // <- empty init-statement
>>>  true)
>>> ;
>>> 
>>> switch (; // empty init-statement
>>>   x) {
>>> ...
>>> }
>>> 
>>> for (; // <- empty init-statement
>>>int y : S())
>>> ;
>>> }
>>> 
>>> As usual, things may or may not go sideways in the presence of macros.
>>> While evaluating this diag on my codebase of interest, it was unsurprisingly
>>> discovered that Google Test macros are *very* prone to this.
>>> And it seems many issues are deep within the GTest itself, not
>>> in the snippets passed from the codebase that uses GTest.
>>> 
>>> So after some thought, i decided not do issue a diagnostic if the semi
>>> is within *any* macro, be it either from the normal header, or system 
>>> header.
>>> 
>>> Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=39111 | PR39111 ]]
>>> 
>>> Reviewers: rsmith, aaron.ballman, efriedma
>>> 
>>> Reviewed By: aaron.ballman
>>> 
>>> Subscribers: cfe-commits
>>> 
>>> Differential Revision: https://reviews.llvm.org/D52695
>>> 
>>> Added:
>>>   
>>> cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp
>>>   cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp
>>> Modified:
>>>   cfe/trunk/docs/ReleaseNotes.rst
>>>   cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>>>   cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>>>   cfe/trunk/include/clang/Parse/Parser.h
>>>   cfe/trunk/lib/Parse/ParseExprCXX.cpp
>>>   cfe/trunk/lib/Parse/ParseStmt.cpp
>>> 
>>> Modified: cfe/trunk/docs/ReleaseNotes.rst
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=347339=347338=347339=diff
>>> ==
>>> --- cfe/trunk/docs/ReleaseNotes.rst (original)
>>> +++ cfe/trunk/docs/ReleaseNotes.rst Tue Nov 20 10:59:05 2018
>>> @@ -55,6 +55,63 @@ Major New Features
>>> Improvements to Clang's diagnostics
>>> ^^^
>>> 
>>> +- ``-Wextra-semi-stmt`` is a new diagnostic that diagnoses extra 
>>> semicolons,
>>> +  much like ``-Wextra-semi``. This new diagnostic diagnoses all 
>>> *unnecessary*
>>> +  null statements (expression statements without an expression), unless: 
>>> the
>>> +  semicolon directly follows a macro that was expanded to nothing or if the
>>> +  semicolon is within the macro itself. This applies to macros defined in 
>>> system
>>> +  headers as well as user-defined macros.
>>> +
>>> +  .. code-block:: c++
>>> +
>>> +  #define MACRO(x) int x;
>>> +  #define NULLMACRO(varname)
>>> +
>>> +  void test() {
>>> +; // <- warning: ';' with no preceding expression is a null 
>>> statement
>>> +
>>> +while (true)
>>> 

[PATCH] D53280: [analyzer] Emit an error for invalid -analyzer-config inputs

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

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

In D53280#1319503 , @NoQ wrote:

> It's a flag that was removed but old soft doesn't know that it was removed, 
> and it won't be fixed because it's old.


We could just add the flag back, and do nothing with it. We have already 
mentioned converting some frontend flags to analyzer configs, and removing some 
long enabled-by-default options while still leaving them in the code in some 
form or another for backward compatibility, so some carefully documented litter 
in `AnalyzerOptions` here and there wouldn't be the end of the world.




Comment at: lib/Driver/ToolChains/Clang.cpp:3307-3309
 void Clang::ConstructJob(Compilation , const JobAction ,
  const InputInfo , const InputInfoList ,
  const ArgList , const char *LinkingOutput) const 
{

NoQ wrote:
> Szelethus wrote:
> > Hmmm. If we just moved the 
> > `CmdArgs.push_back("-analyzer-config-compatibility-mode=true");` line to 
> > this function (which actually invokes `RenderAnalyzerOptions`), that would 
> > solve the issue, wouldn't it? Or somewhere in an even more general driver 
> > function.
> Yeah, but it's not going to be an `AnalyzeJobAction` in the problematic case. 
> So you'd have to add this flag to every single clang invocation.
And I bet that's something we can't do just like that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53280



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

[PATCH] D55269: [CUDA][OpenMP] Fix nvidia-cuda-toolkit detection on Debian/Ubuntu

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

jdenny added a comment.

In D55269#1319452 , @tra wrote:

> In D55269#1319437 , @jdenny wrote:
>
> >
>
>
> That may be a bigger can of works than a single patch can solve. The good 
> news is that the maintainer of CUDA support in cmake is working on adding 
> clang support. So, things may improve soon.


To summarize all this as I understand it: It sounds like anything we do now for 
the cmake issue would be a short-term solution because cmake might later 
present a better solution.  Originally, I figured the easiest place to work 
around this issue was clang as that would help anyone who detects a CUDA 
installation using current cmake support.  You prefer to do it in OpenMP's 
cmake files, which I suppose would help avoid any unexpected consequences from 
my change to clang.  A third alternative is to just wait for cmake to improve.

For now, I'll trim this patch to just the IsUbuntu() part (maybe tomorrow), and 
we can think more about the cmake side of things.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55269



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

Re: r346041 - Diagnose parameter names that shadow the names of inherited fields under -Wshadow-field.

[George Karpenkov via cfe-commits]

> On Dec 4, 2018, at 4:48 PM, Aaron Ballman  wrote:
> 
> On Tue, Dec 4, 2018 at 7:17 PM George Karpenkov  wrote:
>> 
>> Hi Aaron,
>> 
>> Should such changes be reviewed?
> 
> This was reviewed in D52421.
> 
>> In particular, it introduces tons of warnings in XNU (which are painful due 
>> to -Werror).
>> I expect similar issues in many other our projects.
>> 
>> While in principle correct,
>> wouldn’t it make more sense to only warn on function definitions and not 
>> declarations?
> 
> Absolutely! I'll make that change, thank you for the suggestion.

OK thanks!

> 
> ~Aaron
> 
>> 
>> This would avoid:
>> 
>> - A gigantic avalanche of warnings when compiling a project,
>> as the warning would be duplicated for every TU including the problematic 
>> header
>> - A warning on declaration without a body is  useful: nothing can collide 
>> there,
>> and the actual definition might use a different name.
>> 
>> George
>> 
>>> On Nov 2, 2018, at 2:04 PM, Aaron Ballman via cfe-commits 
>>>  wrote:
>>> 
>>> Author: aaronballman
>>> Date: Fri Nov  2 14:04:44 2018
>>> New Revision: 346041
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=346041=rev
>>> Log:
>>> Diagnose parameter names that shadow the names of inherited fields under 
>>> -Wshadow-field.
>>> 
>>> This addresses PR34120. Note, unlike GCC, we take into account the 
>>> accessibility of the field when deciding whether to warn or not.
>>> 
>>> Modified:
>>>   cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>>   cfe/trunk/include/clang/Sema/Sema.h
>>>   cfe/trunk/lib/Sema/SemaDecl.cpp
>>>   cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>>>   cfe/trunk/test/SemaCXX/warn-shadow.cpp
>>> 
>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=346041=346040=346041=diff
>>> ==
>>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Nov  2 
>>> 14:04:44 2018
>>> @@ -9467,16 +9467,15 @@ def warn_block_literal_qualifiers_on_omi
>>>  InGroup;
>>> 
>>> def ext_warn_gnu_final : ExtWarn<
>>> -  "__final is a GNU extension, consider using C++11 final">,
>>> -  InGroup;
>>> -
>>> -def warn_shadow_field :
>>> -  Warning<"non-static data member %0 of %1 shadows member inherited from "
>>> -  "type %2">,
>>> -  InGroup, DefaultIgnore;
>>> -def note_shadow_field : Note<"declared here">;
>>> -
>>> -def err_multiversion_required_in_redecl : Error<
>>> +  "__final is a GNU extension, consider using C++11 final">,
>>> +  InGroup;
>>> +
>>> +def warn_shadow_field : Warning<
>>> +  "%select{parameter|non-static data member}3 %0 %select{|of %1 }3shadows "
>>> +  "member inherited from type %2">, InGroup, DefaultIgnore;
>>> +def note_shadow_field : Note<"declared here">;
>>> +
>>> +def err_multiversion_required_in_redecl : Error<
>>>  "function declaration is missing %select{'target'|'cpu_specific' or "
>>>  "'cpu_dispatch'}0 attribute in a multiversioned function">;
>>> def note_multiversioning_caused_here : Note<
>>> 
>>> Modified: cfe/trunk/include/clang/Sema/Sema.h
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=346041=346040=346041=diff
>>> ==
>>> --- cfe/trunk/include/clang/Sema/Sema.h (original)
>>> +++ cfe/trunk/include/clang/Sema/Sema.h Fri Nov  2 14:04:44 2018
>>> @@ -10588,7 +10588,8 @@ private:
>>>  /// Check if there is a field shadowing.
>>>  void CheckShadowInheritedFields(const SourceLocation ,
>>>  DeclarationName FieldName,
>>> -  const CXXRecordDecl *RD);
>>> +  const CXXRecordDecl *RD,
>>> +  bool DeclIsField = true);
>>> 
>>>  /// Check if the given expression contains 'break' or 'continue'
>>>  /// statement that produces control flow different from GCC.
>>> 
>>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=346041=346040=346041=diff
>>> ==
>>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Nov  2 14:04:44 2018
>>> @@ -12366,6 +12366,13 @@ Decl *Sema::ActOnParamDeclarator(Scope *
>>>D.setInvalidType(true);
>>>  }
>>>}
>>> +
>>> +if (LangOpts.CPlusPlus) {
>>> +  DeclarationNameInfo DNI = GetNameForDeclarator(D);
>>> +  if (auto *RD = dyn_cast(CurContext))
>>> +CheckShadowInheritedFields(DNI.getLoc(), DNI.getName(), RD,
>>> +   /*DeclIsField*/ false);
>>> +}
>>>  }
>>> 
>>>  // Temporarily put parameter variables in the translation unit, not
>>> 
>>> Modified: 

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

[Gábor Horváth via Phabricator via cfe-commits]

hgabii updated this revision to Diff 176736.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D42682

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/IoFunctionsCheck.cpp
  clang-tidy/bugprone/IoFunctionsCheck.h
  clang-tidy/cert/CERTTidyModule.cpp
  clang-tidy/cert/CMakeLists.txt
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-io-functions.rst
  docs/clang-tidy/checks/cert-fio34-c.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-io-functions.cpp

Index: test/clang-tidy/bugprone-io-functions.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-io-functions.cpp
@@ -0,0 +1,121 @@
+// RUN: %check_clang_tidy %s bugprone-io-functions %t
+
+typedef int wint_t;
+typedef void *FILE;
+
+namespace std {
+int getchar();
+int getc(FILE);
+int fgetc(FILE);
+wint_t getwchar();
+wint_t getwc(FILE);
+wint_t fgetwc(FILE);
+class istream {
+public:
+  wint_t get();
+};
+} // namespace std
+
+int char_io_getchar() {
+  char c;
+  c = std::getchar();
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'getchar' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+  return c;
+}
+
+int char_io_getc(FILE *fp) {
+  char c;
+  c = std::getc(fp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'getc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+  return c;
+}
+
+int char_io_fgetc(FILE *fp) {
+  char c;
+  c = std::fgetc(fp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'fgetc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+  return c;
+}
+
+int char_io_getwchar() {
+  char c;
+  c = std::getwchar();
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'getwchar' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+  return c;
+}
+
+int char_io_getwc(FILE *fp) {
+  char c;
+  c = std::getwc(fp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'getwc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+  return c;
+}
+
+int char_io_fgetwc(FILE *fp) {
+  char c;
+  c = std::fgetwc(fp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'fgetwc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+  return c;
+}
+
+int char_io_get(std::istream ) {
+  char c;
+  c = is.get();
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'get' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+  return c;
+}
+
+void char_type_in_argument(char c) {}
+
+void call_char_type_in_argument_with_getchar(FILE *fp) {
+  char_type_in_argument(std::fgetc(fp));
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: consider to cast the return value of 'fgetc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+}
+
+void call_char_type_in_argument_with_getwchar() {
+  char_type_in_argument(std::getwchar());
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: consider to cast the return value of 'getwchar' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+}
+
+// Negatives
+int int_io_getchar() {
+  int x;
+  x = std::getchar();
+  return x;
+}
+
+int int_io_getc(FILE *fp) {
+  int x;
+  x = std::getc(fp);
+  return x;
+}
+
+int int_io_fgetc(FILE *fp) {
+  int x;
+  x = std::fgetc(fp);
+  return x;
+}
+
+int int_io_getwchar() {
+  int x;
+  x = std::getwchar();
+  return x;
+}
+
+int int_io_getwc(FILE *fp) {
+  int x;
+  x = std::getwc(fp);
+  return x;
+}
+
+int int_io_fgetwc(FILE *fp) {
+  int x;
+  x = std::fgetwc(fp);
+  return x;
+}
+
+int int_io_get(std::istream ) {
+  int x;
+  x = is.get();
+  return x;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -31,6 +31,7 @@
bugprone-inaccurate-erase
bugprone-incorrect-roundings
bugprone-integer-division
+   bugprone-io-functions
bugprone-lambda-function-name

[PATCH] D53280: [analyzer] Emit an error for invalid -analyzer-config inputs

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

It's a flag that was removed but old soft doesn't know that it was removed, and 
it won't be fixed because it's old. I guess a typo would have caused the same 
problem.

Not urgent, no pressure. Yes, i believe this patch is good and should stay 
there. At the same time, by the looks of it, coming up with a fix for this 
"problem" may be annoying.




Comment at: lib/Driver/ToolChains/Clang.cpp:3307-3309
 void Clang::ConstructJob(Compilation , const JobAction ,
  const InputInfo , const InputInfoList ,
  const ArgList , const char *LinkingOutput) const 
{

Szelethus wrote:
> Hmmm. If we just moved the 
> `CmdArgs.push_back("-analyzer-config-compatibility-mode=true");` line to this 
> function (which actually invokes `RenderAnalyzerOptions`), that would solve 
> the issue, wouldn't it? Or somewhere in an even more general driver function.
Yeah, but it's not going to be an `AnalyzeJobAction` in the problematic case. 
So you'd have to add this flag to every single clang invocation.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53280



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

Re: r346041 - Diagnose parameter names that shadow the names of inherited fields under -Wshadow-field.

[Aaron Ballman via cfe-commits]

On Tue, Dec 4, 2018 at 7:17 PM George Karpenkov  wrote:
>
> Hi Aaron,
>
> Should such changes be reviewed?

This was reviewed in D52421.

> In particular, it introduces tons of warnings in XNU (which are painful due 
> to -Werror).
> I expect similar issues in many other our projects.
>
> While in principle correct,
> wouldn’t it make more sense to only warn on function definitions and not 
> declarations?

Absolutely! I'll make that change, thank you for the suggestion.

~Aaron

>
> This would avoid:
>
>  - A gigantic avalanche of warnings when compiling a project,
> as the warning would be duplicated for every TU including the problematic 
> header
>  - A warning on declaration without a body is  useful: nothing can collide 
> there,
> and the actual definition might use a different name.
>
> George
>
> > On Nov 2, 2018, at 2:04 PM, Aaron Ballman via cfe-commits 
> >  wrote:
> >
> > Author: aaronballman
> > Date: Fri Nov  2 14:04:44 2018
> > New Revision: 346041
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=346041=rev
> > Log:
> > Diagnose parameter names that shadow the names of inherited fields under 
> > -Wshadow-field.
> >
> > This addresses PR34120. Note, unlike GCC, we take into account the 
> > accessibility of the field when deciding whether to warn or not.
> >
> > Modified:
> >cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >cfe/trunk/include/clang/Sema/Sema.h
> >cfe/trunk/lib/Sema/SemaDecl.cpp
> >cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> >cfe/trunk/test/SemaCXX/warn-shadow.cpp
> >
> > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> > URL: 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=346041=346040=346041=diff
> > ==
> > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Nov  2 
> > 14:04:44 2018
> > @@ -9467,16 +9467,15 @@ def warn_block_literal_qualifiers_on_omi
> >   InGroup;
> >
> > def ext_warn_gnu_final : ExtWarn<
> > -  "__final is a GNU extension, consider using C++11 final">,
> > -  InGroup;
> > -
> > -def warn_shadow_field :
> > -  Warning<"non-static data member %0 of %1 shadows member inherited from "
> > -  "type %2">,
> > -  InGroup, DefaultIgnore;
> > -def note_shadow_field : Note<"declared here">;
> > -
> > -def err_multiversion_required_in_redecl : Error<
> > +  "__final is a GNU extension, consider using C++11 final">,
> > +  InGroup;
> > +
> > +def warn_shadow_field : Warning<
> > +  "%select{parameter|non-static data member}3 %0 %select{|of %1 }3shadows "
> > +  "member inherited from type %2">, InGroup, DefaultIgnore;
> > +def note_shadow_field : Note<"declared here">;
> > +
> > +def err_multiversion_required_in_redecl : Error<
> >   "function declaration is missing %select{'target'|'cpu_specific' or "
> >   "'cpu_dispatch'}0 attribute in a multiversioned function">;
> > def note_multiversioning_caused_here : Note<
> >
> > Modified: cfe/trunk/include/clang/Sema/Sema.h
> > URL: 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=346041=346040=346041=diff
> > ==
> > --- cfe/trunk/include/clang/Sema/Sema.h (original)
> > +++ cfe/trunk/include/clang/Sema/Sema.h Fri Nov  2 14:04:44 2018
> > @@ -10588,7 +10588,8 @@ private:
> >   /// Check if there is a field shadowing.
> >   void CheckShadowInheritedFields(const SourceLocation ,
> >   DeclarationName FieldName,
> > -  const CXXRecordDecl *RD);
> > +  const CXXRecordDecl *RD,
> > +  bool DeclIsField = true);
> >
> >   /// Check if the given expression contains 'break' or 'continue'
> >   /// statement that produces control flow different from GCC.
> >
> > Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=346041=346040=346041=diff
> > ==
> > --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> > +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Nov  2 14:04:44 2018
> > @@ -12366,6 +12366,13 @@ Decl *Sema::ActOnParamDeclarator(Scope *
> > D.setInvalidType(true);
> >   }
> > }
> > +
> > +if (LangOpts.CPlusPlus) {
> > +  DeclarationNameInfo DNI = GetNameForDeclarator(D);
> > +  if (auto *RD = dyn_cast(CurContext))
> > +CheckShadowInheritedFields(DNI.getLoc(), DNI.getName(), RD,
> > +   /*DeclIsField*/ false);
> > +}
> >   }
> >
> >   // Temporarily put parameter variables in the translation unit, not
> >
> > Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> > URL: 
> > 

Re: r347339 - [clang][Parse] Diagnose useless null statements / empty init-statements

[Aaron Ballman via cfe-commits]

On Tue, Dec 4, 2018 at 7:35 PM George Karpenkov  wrote:
>
> Hi Roman,
>
> I’m against this new warning.
>
> A very common idiom is defining a “DEBUG” macro to be a no-op in release, and 
> a logging function otherwise.
> The new introduced warning warns on all such cases (e.g. 'DEBUG(“mystring”);')

Yeah, that's not good.

> As noted in the review, Clang warnings should generally not be used to 
> diagnose style issues — and an extra semicolon does not seem to be a common 
> bug source.
>
> It is a problem in practice for us since we have projects which enable all 
> available warnings and also enable “-Werror”.

Would you be okay with the warning if it was only diagnosed when the
source location of the semi-colon is not immediately preceded by a
macro expansion location? e.g.,

EMPTY_EXPANSION(12); // Not diagnosed
EMPTY_EXPANSION; // Not diagnosed
; // Diagnosed

~Aaron

>
> Regards,
> George
>
>
> > On Nov 20, 2018, at 10:59 AM, Roman Lebedev via cfe-commits 
> >  wrote:
> >
> > Author: lebedevri
> > Date: Tue Nov 20 10:59:05 2018
> > New Revision: 347339
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=347339=rev
> > Log:
> > [clang][Parse] Diagnose useless null statements / empty init-statements
> >
> > Summary:
> > clang has `-Wextra-semi` (D43162), which is not dictated by the currently 
> > selected standard.
> > While that is great, there is at least one more source of need-less semis - 
> > 'null statements'.
> > Sometimes, they are needed:
> > ```
> > for(int x = 0; continueToDoWork(x); x++)
> >  ; // Ugly code, but the semi is needed here.
> > ```
> >
> > But sometimes they are just there for no reason:
> > ```
> > switch(X) {
> > case 0:
> >  return -2345;
> > case 5:
> >  return 0;
> > default:
> >  return 42;
> > }; // <- oops
> >
> > ;;; <- PS, still not diagnosed. Clearly this is junk.
> > ```
> >
> > Additionally:
> > ```
> > if(; // <- empty init-statement
> >   true)
> >  ;
> >
> > switch (; // empty init-statement
> >x) {
> >  ...
> > }
> >
> > for (; // <- empty init-statement
> > int y : S())
> >  ;
> > }
> >
> > As usual, things may or may not go sideways in the presence of macros.
> > While evaluating this diag on my codebase of interest, it was unsurprisingly
> > discovered that Google Test macros are *very* prone to this.
> > And it seems many issues are deep within the GTest itself, not
> > in the snippets passed from the codebase that uses GTest.
> >
> > So after some thought, i decided not do issue a diagnostic if the semi
> > is within *any* macro, be it either from the normal header, or system 
> > header.
> >
> > Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=39111 | PR39111 ]]
> >
> > Reviewers: rsmith, aaron.ballman, efriedma
> >
> > Reviewed By: aaron.ballman
> >
> > Subscribers: cfe-commits
> >
> > Differential Revision: https://reviews.llvm.org/D52695
> >
> > Added:
> >
> > cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp
> >cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp
> > Modified:
> >cfe/trunk/docs/ReleaseNotes.rst
> >cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> >cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
> >cfe/trunk/include/clang/Parse/Parser.h
> >cfe/trunk/lib/Parse/ParseExprCXX.cpp
> >cfe/trunk/lib/Parse/ParseStmt.cpp
> >
> > Modified: cfe/trunk/docs/ReleaseNotes.rst
> > URL: 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=347339=347338=347339=diff
> > ==
> > --- cfe/trunk/docs/ReleaseNotes.rst (original)
> > +++ cfe/trunk/docs/ReleaseNotes.rst Tue Nov 20 10:59:05 2018
> > @@ -55,6 +55,63 @@ Major New Features
> > Improvements to Clang's diagnostics
> > ^^^
> >
> > +- ``-Wextra-semi-stmt`` is a new diagnostic that diagnoses extra 
> > semicolons,
> > +  much like ``-Wextra-semi``. This new diagnostic diagnoses all 
> > *unnecessary*
> > +  null statements (expression statements without an expression), unless: 
> > the
> > +  semicolon directly follows a macro that was expanded to nothing or if the
> > +  semicolon is within the macro itself. This applies to macros defined in 
> > system
> > +  headers as well as user-defined macros.
> > +
> > +  .. code-block:: c++
> > +
> > +  #define MACRO(x) int x;
> > +  #define NULLMACRO(varname)
> > +
> > +  void test() {
> > +; // <- warning: ';' with no preceding expression is a null 
> > statement
> > +
> > +while (true)
> > +  ; // OK, it is needed.
> > +
> > +switch (my_enum) {
> > +case E1:
> > +  // stuff
> > +  break;
> > +case E2:
> > +  ; // OK, it is needed.
> > +}
> > +
> > +MACRO(v0;) // Extra semicolon, but within macro, so ignored.
> > +
> > +MACRO(v1); // <- warning: ';' with no preceding expression is a 
> > null statement
> > +
> > +

[PATCH] D53280: [analyzer] Emit an error for invalid -analyzer-config inputs

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

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



Comment at: lib/Driver/ToolChains/Clang.cpp:3307-3309
 void Clang::ConstructJob(Compilation , const JobAction ,
  const InputInfo , const InputInfoList ,
  const ArgList , const char *LinkingOutput) const 
{

Hmmm. If we just moved the 
`CmdArgs.push_back("-analyzer-config-compatibility-mode=true");` line to this 
function (which actually invokes `RenderAnalyzerOptions`), that would solve the 
issue, wouldn't it? Or somewhere in an even more general driver function.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53280



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

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/bugprone-io-functions.rst:4
+bugprone-io-functions
+=
+

Please adjust length.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D42682



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

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno updated this revision to Diff 176733.
bruno added a comment.

Update patch after @erik.pilkington review!


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

https://reviews.llvm.org/D55097

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
  test/CXX/drs/dr6xx.cpp
  www/cxx_status.html

Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -953,13 +953,15 @@
 
   Relaxations of constexpr restrictions
   http://wg21.link/p1064r0;>P1064R0
-  No
+  No
 

 http://wg21.link/p1002r1;>P1002R1
+SVN
   
   
 http://wg21.link/p1327r1;>P1327R1
+No
   
   
 http://wg21.link/p1330r0;>P1330R0
Index: test/CXX/drs/dr6xx.cpp
===
--- test/CXX/drs/dr6xx.cpp
+++ test/CXX/drs/dr6xx.cpp
@@ -492,7 +492,13 @@
   struct C {
 constexpr C(NonLiteral);
 constexpr C(NonLiteral, int) {} // expected-error {{not a literal type}}
-constexpr C() try {} catch (...) {} // expected-error {{function try block}}
+constexpr C() try {} catch (...) {}
+#if __cplusplus <= 201703L
+// expected-error@-2 {{function try block in constexpr constructor is a C++2a extension}}
+#endif
+#if __cplusplus < 201402L
+// expected-error@-5 {{use of this statement in a constexpr constructor is a C++14 extension}}
+#endif
   };
 
   struct D {
Index: test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
===
--- test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
+++ test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -verify -std=c++11 -fcxx-exceptions -Werror=c++1y-extensions %s
-// RUN: %clang_cc1 -verify -std=c++1y -fcxx-exceptions -DCXX1Y %s
+// RUN: %clang_cc1 -verify -std=c++11 -fcxx-exceptions -Werror=c++1y-extensions -Werror=c++2a-extensions %s
+// RUN: %clang_cc1 -verify -std=c++1y -fcxx-exceptions -DCXX1Y -Werror=c++2a-extensions %s
+// RUN: %clang_cc1 -verify -std=c++2a -fcxx-exceptions -DCXX1Y -DCXX2A %s
 
 namespace N {
   typedef char C;
@@ -49,8 +50,14 @@
 // - its function-body shall not be a function-try-block;
 struct U {
   constexpr U()
-try // expected-error {{function try block not allowed in constexpr constructor}}
+try
+#ifndef CXX2A
+  // expected-error@-2 {{function try block in constexpr constructor is a C++2a extension}}
+#endif
 : u() {
+#ifndef CXX1Y
+  // expected-error@-2 {{use of this statement in a constexpr constructor is a C++14 extension}}
+#endif
   } catch (...) {
 throw;
   }
Index: test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++11 -Werror=c++1y-extensions %s
-// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++1y -DCXX1Y %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++11 -Werror=c++1y-extensions -Werror=c++2a-extensions %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++1y -DCXX1Y -Werror=c++2a-extensions %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++2a -DCXX1Y -DCXX2A %s
 
 namespace N {
   typedef char C;
@@ -78,7 +79,12 @@
 };
 struct T3 {
   constexpr T3 =(const T3&) const = default;
-  // expected-error@-1 {{an explicitly-defaulted copy assignment operator may not have 'const' or 'volatile' qualifiers}}
+#ifndef CXX2A
+  // expected-error@-2 {{an explicitly-defaulted copy assignment operator may not have 'const' or 'volatile' qualifiers}}
+#else
+  // expected-warning@-4 {{explicitly defaulted copy assignment operator is implicitly deleted}}
+  // expected-note@-5 {{function is implicitly deleted because its declared type does not match the type of an implicit copy assignment operator}}
+#endif
 };
 #endif
 struct U {
@@ -131,7 +137,13 @@
 }
 constexpr int DisallowedStmtsCXX1Y_3() {
   //  - a try-block,
-  try {} catch (...) {} // expected-error {{statement not allowed in constexpr function}}
+  try {} catch (...) {}
+#ifndef CXX2A
+  // expected-error@-2 {{use of this statement in a constexpr function is a C++2a extension}}
+#ifndef CXX1Y
+  // expected-error@-4 {{use of this statement in a constexpr function is a C++14 extension}}
+#endif
+#endif
   return 0;
 }
 constexpr int DisallowedStmtsCXX1Y_4() {
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -1803,7 +1803,7 @@
 static bool
 

r348325 - [Sema] Remove some conditions of a failing assert

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Tue Dec  4 16:43:11 2018
New Revision: 348325

URL: http://llvm.org/viewvc/llvm-project?rev=348325=rev
Log:
[Sema] Remove some conditions of a failing assert

We should have been checking that this state is consistent, but its
possible for it to be filled later, so it isn't really sound to check
it here anyways.

Fixes llvm.org/PR39742

Modified:
cfe/trunk/lib/AST/DeclTemplate.cpp
cfe/trunk/test/SemaCXX/friend-template-redecl.cpp

Modified: cfe/trunk/lib/AST/DeclTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclTemplate.cpp?rev=348325=348324=348325=diff
==
--- cfe/trunk/lib/AST/DeclTemplate.cpp (original)
+++ cfe/trunk/lib/AST/DeclTemplate.cpp Tue Dec  4 16:43:11 2018
@@ -329,8 +329,6 @@ void FunctionTemplateDecl::mergePrevDecl
 
   // Ensure we don't leak any important state.
   assert(ThisCommon->Specializations.size() == 0 &&
- !ThisCommon->InstantiatedFromMember.getPointer() &&
- !ThisCommon->InstantiatedFromMember.getInt() &&
  "Can't merge incompatible declarations!");
 
   Base::Common = PrevCommon;

Modified: cfe/trunk/test/SemaCXX/friend-template-redecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/friend-template-redecl.cpp?rev=348325=348324=348325=diff
==
--- cfe/trunk/test/SemaCXX/friend-template-redecl.cpp (original)
+++ cfe/trunk/test/SemaCXX/friend-template-redecl.cpp Tue Dec  4 16:43:11 2018
@@ -18,3 +18,14 @@ void f() {
   foo(x);
   bar(x);
 }
+
+namespace PR39742 {
+template
+struct wrapper {
+  template
+  friend void friend_function_template() {}
+};
+
+wrapper x;
+wrapper y;
+}


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

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

[Gábor Horváth via Phabricator via cfe-commits]

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

I moved to bugprone module and renamed to bugprone-io-functions.
I added as a reference for cert-fio34-c.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D42682



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

Re: r347339 - [clang][Parse] Diagnose useless null statements / empty init-statements

[George Karpenkov via cfe-commits]

Hi Roman,

I’m against this new warning.

A very common idiom is defining a “DEBUG” macro to be a no-op in release, and a 
logging function otherwise.
The new introduced warning warns on all such cases (e.g. 'DEBUG(“mystring”);')

As noted in the review, Clang warnings should generally not be used to diagnose 
style issues — and an extra semicolon does not seem to be a common bug source.

It is a problem in practice for us since we have projects which enable all 
available warnings and also enable “-Werror”.

Regards,
George


> On Nov 20, 2018, at 10:59 AM, Roman Lebedev via cfe-commits 
>  wrote:
> 
> Author: lebedevri
> Date: Tue Nov 20 10:59:05 2018
> New Revision: 347339
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=347339=rev
> Log:
> [clang][Parse] Diagnose useless null statements / empty init-statements
> 
> Summary:
> clang has `-Wextra-semi` (D43162), which is not dictated by the currently 
> selected standard.
> While that is great, there is at least one more source of need-less semis - 
> 'null statements'.
> Sometimes, they are needed:
> ```
> for(int x = 0; continueToDoWork(x); x++)
>  ; // Ugly code, but the semi is needed here.
> ```
> 
> But sometimes they are just there for no reason:
> ```
> switch(X) {
> case 0:
>  return -2345;
> case 5:
>  return 0;
> default:
>  return 42;
> }; // <- oops
> 
> ;;; <- PS, still not diagnosed. Clearly this is junk.
> ```
> 
> Additionally:
> ```
> if(; // <- empty init-statement
>   true)
>  ;
> 
> switch (; // empty init-statement
>x) {
>  ...
> }
> 
> for (; // <- empty init-statement
> int y : S())
>  ;
> }
> 
> As usual, things may or may not go sideways in the presence of macros.
> While evaluating this diag on my codebase of interest, it was unsurprisingly
> discovered that Google Test macros are *very* prone to this.
> And it seems many issues are deep within the GTest itself, not
> in the snippets passed from the codebase that uses GTest.
> 
> So after some thought, i decided not do issue a diagnostic if the semi
> is within *any* macro, be it either from the normal header, or system header.
> 
> Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=39111 | PR39111 ]]
> 
> Reviewers: rsmith, aaron.ballman, efriedma
> 
> Reviewed By: aaron.ballman
> 
> Subscribers: cfe-commits
> 
> Differential Revision: https://reviews.llvm.org/D52695
> 
> Added:
>
> cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt-in-init-statement.cpp
>cfe/trunk/test/Parser/extra-semi-resulting-in-nullstmt.cpp
> Modified:
>cfe/trunk/docs/ReleaseNotes.rst
>cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>cfe/trunk/include/clang/Parse/Parser.h
>cfe/trunk/lib/Parse/ParseExprCXX.cpp
>cfe/trunk/lib/Parse/ParseStmt.cpp
> 
> Modified: cfe/trunk/docs/ReleaseNotes.rst
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=347339=347338=347339=diff
> ==
> --- cfe/trunk/docs/ReleaseNotes.rst (original)
> +++ cfe/trunk/docs/ReleaseNotes.rst Tue Nov 20 10:59:05 2018
> @@ -55,6 +55,63 @@ Major New Features
> Improvements to Clang's diagnostics
> ^^^
> 
> +- ``-Wextra-semi-stmt`` is a new diagnostic that diagnoses extra semicolons,
> +  much like ``-Wextra-semi``. This new diagnostic diagnoses all *unnecessary*
> +  null statements (expression statements without an expression), unless: the
> +  semicolon directly follows a macro that was expanded to nothing or if the
> +  semicolon is within the macro itself. This applies to macros defined in 
> system
> +  headers as well as user-defined macros.
> +
> +  .. code-block:: c++
> +
> +  #define MACRO(x) int x;
> +  #define NULLMACRO(varname)
> +
> +  void test() {
> +; // <- warning: ';' with no preceding expression is a null statement
> +
> +while (true)
> +  ; // OK, it is needed.
> +
> +switch (my_enum) {
> +case E1:
> +  // stuff
> +  break;
> +case E2:
> +  ; // OK, it is needed.
> +}
> +
> +MACRO(v0;) // Extra semicolon, but within macro, so ignored.
> +
> +MACRO(v1); // <- warning: ';' with no preceding expression is a null 
> statement
> +
> +NULLMACRO(v2); // ignored, NULLMACRO expanded to nothing.
> +  }
> +
> +- ``-Wempty-init-stmt`` is a new diagnostic that diagnoses empty 
> init-statements
> +  of ``if``, ``switch``, ``range-based for``, unless: the semicolon directly
> +  follows a macro that was expanded to nothing or if the semicolon is within 
> the
> +  macro itself (both macros from system headers, and normal macros). This
> +  diagnostic is in the ``-Wextra-semi-stmt`` group and is enabled in
> +  ``-Wextra``.
> +
> +  .. code-block:: c++
> +
> +  void test() {
> +if(; // <- warning: init-statement of 'if' is a null statement
> +   true)

[PATCH] D53280: [analyzer] Emit an error for invalid -analyzer-config inputs

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

Szelethus added a comment.

In D53280#1319446 , @NoQ wrote:

> Ugh. We broke backwards compatibility when an invalid `-analyzer-config` is 
> specified but `-analyze` isn't specified. Yes, people are accidentally 
> relying on that :/ :/ :/
>
> In this case the driver doesn't know that it needs to add 
> `analyzer-config-compatibility-mode=true` (or, even better, outright ignore 
> all analyzer options) but the frontend doesn't know that it doesn't need to 
> parse them.


Oh my god. Can you go a little more in depth? Was the problem something along 
the lines of `treu` typo, or a non-existent flag? Is that at all salvageable? I 
mean, losing the entire patch would be super bad imo.

Also, I can't attend to this issue immediately -- replying on the phone is one 
thing, but almost any other action is out of my reach for the next couple 
days/weeks.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53280



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

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

[Gábor Horváth via Phabricator via cfe-commits]

hgabii updated this revision to Diff 176732.
hgabii marked an inline comment as done.
hgabii edited the summary of this revision.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D42682

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/IoFunctionsCheck.cpp
  clang-tidy/bugprone/IoFunctionsCheck.h
  clang-tidy/cert/CERTTidyModule.cpp
  clang-tidy/cert/CMakeLists.txt
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-io-functions.rst
  docs/clang-tidy/checks/cert-fio34-c.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-io-functions.cpp

Index: test/clang-tidy/bugprone-io-functions.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-io-functions.cpp
@@ -0,0 +1,121 @@
+// RUN: %check_clang_tidy %s bugprone-io-functions %t
+
+typedef int wint_t;
+typedef void *FILE;
+
+namespace std {
+int getchar();
+int getc(FILE);
+int fgetc(FILE);
+wint_t getwchar();
+wint_t getwc(FILE);
+wint_t fgetwc(FILE);
+class istream {
+public:
+  wint_t get();
+};
+} // namespace std
+
+int char_io_getchar() {
+  char c;
+  c = std::getchar();
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'getchar' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+  return c;
+}
+
+int char_io_getc(FILE *fp) {
+  char c;
+  c = std::getc(fp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'getc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+  return c;
+}
+
+int char_io_fgetc(FILE *fp) {
+  char c;
+  c = std::fgetc(fp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'fgetc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+  return c;
+}
+
+int char_io_getwchar() {
+  char c;
+  c = std::getwchar();
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'getwchar' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+  return c;
+}
+
+int char_io_getwc(FILE *fp) {
+  char c;
+  c = std::getwc(fp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'getwc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+  return c;
+}
+
+int char_io_fgetwc(FILE *fp) {
+  char c;
+  c = std::fgetwc(fp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'fgetwc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+  return c;
+}
+
+int char_io_get(std::istream ) {
+  char c;
+  c = is.get();
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'get' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+  return c;
+}
+
+void char_type_in_argument(char c) {}
+
+void call_char_type_in_argument_with_getchar(FILE *fp) {
+  char_type_in_argument(std::fgetc(fp));
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: consider to cast the return value of 'fgetc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+}
+
+void call_char_type_in_argument_with_getwchar() {
+  char_type_in_argument(std::getwchar());
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: consider to cast the return value of 'getwchar' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached [bugprone-io-functions]
+}
+
+// Negatives
+int int_io_getchar() {
+  int x;
+  x = std::getchar();
+  return x;
+}
+
+int int_io_getc(FILE *fp) {
+  int x;
+  x = std::getc(fp);
+  return x;
+}
+
+int int_io_fgetc(FILE *fp) {
+  int x;
+  x = std::fgetc(fp);
+  return x;
+}
+
+int int_io_getwchar() {
+  int x;
+  x = std::getwchar();
+  return x;
+}
+
+int int_io_getwc(FILE *fp) {
+  int x;
+  x = std::getwc(fp);
+  return x;
+}
+
+int int_io_fgetwc(FILE *fp) {
+  int x;
+  x = std::fgetwc(fp);
+  return x;
+}
+
+int int_io_get(std::istream ) {
+  int x;
+  x = is.get();
+  return x;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -31,6 +31,7 @@
bugprone-inaccurate-erase
bugprone-incorrect-roundings

[PATCH] D55269: [CUDA][OpenMP] Fix nvidia-cuda-toolkit detection on Debian/Ubuntu

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D55269#1319437 , @jdenny wrote:

> In D55269#1319382 , @tra wrote:
>
> > Let's start with fixing OpenMP's cmake files. Once it no longer insists on 
> > specifying --cuda-path=/usr, and isUbuntu is in place, what is the 
> > remaining failure that you see?
>
>
> I'm fairly certain I would see no remaining failure then, but it's not clear 
> to me how we should convince OpenMP's cmake files to stop doing that.
>
> openmp/libomptarget/cmake/Modules/LibomptargetNVPTXBitcodeLibrary.cmake is 
> the file, and there's one occurrence of --cuda-path.
>
> If CUDA_TOOLKIT_ROOT_DIR is specified explicitly to cmake, that --cuda-path 
> is necessary, so we cannot just remove it.
>
> If CUDA_TOOLKIT_ROOT_DIR is not specified explicitly, it is computed by 
> cmake.  So I believe this boils down to getting cmake to find the right CUDA 
> root on Debian/Ubuntu.
>
> Do you agree so far?


I guess we're dealing with even more  issues here.
cmake currently only knows about CUDA compilation with nvcc. The autodetection 
of CUDA_TOOLKIT_ROOT_DIR is intended to work for nvcc only and it does serve 
that purpose. Debian/Ubuntu packaging was also initially done with only nvcc in 
mind and it does work for nvcc. OpenMP's cmake files rely on 
CUDA_TOOLKIT_ROOT_DIR to find CUDA SDK and that is what fails for clang in case 
of split CUDA SDK installation.

One way to deal with that would be to change OpenMP's cmake files to check 
whether we're using Clang to compile CUDA and, if that's the case override CUDA 
root detection. Maybe ensure that CUDA install is monolithic and require user 
to specify a different location. Or check if we've detected a split path in 
/usr and override it with /usr/lib/cuda.

> Would you recommend submitting a patch to cmake's CUDA support?  Or would you 
> recommend replicating clang's logic from D40453 
>  into openmp's cmake files, overriding 
> cmake's own selection of CUDA_TOOLKIT_ROOT_DIR?

That may be a bigger can of works than a single patch can solve. The good news 
is that the maintainer of CUDA support in cmake is working on adding clang 
support. So, things may improve soon.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55269



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

Re: r346041 - Diagnose parameter names that shadow the names of inherited fields under -Wshadow-field.

[George Karpenkov via cfe-commits]

Hi Aaron,

Should such changes be reviewed?
In particular, it introduces tons of warnings in XNU (which are painful due to 
-Werror).
I expect similar issues in many other our projects.

While in principle correct,
wouldn’t it make more sense to only warn on function definitions and not 
declarations?

This would avoid:

 - A gigantic avalanche of warnings when compiling a project,
as the warning would be duplicated for every TU including the problematic header
 - A warning on declaration without a body is  useful: nothing can collide 
there,
and the actual definition might use a different name.

George

> On Nov 2, 2018, at 2:04 PM, Aaron Ballman via cfe-commits 
>  wrote:
> 
> Author: aaronballman
> Date: Fri Nov  2 14:04:44 2018
> New Revision: 346041
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=346041=rev
> Log:
> Diagnose parameter names that shadow the names of inherited fields under 
> -Wshadow-field.
> 
> This addresses PR34120. Note, unlike GCC, we take into account the 
> accessibility of the field when deciding whether to warn or not.
> 
> Modified:
>cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>cfe/trunk/include/clang/Sema/Sema.h
>cfe/trunk/lib/Sema/SemaDecl.cpp
>cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>cfe/trunk/test/SemaCXX/warn-shadow.cpp
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=346041=346040=346041=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Nov  2 14:04:44 
> 2018
> @@ -9467,16 +9467,15 @@ def warn_block_literal_qualifiers_on_omi
>   InGroup;
> 
> def ext_warn_gnu_final : ExtWarn<
> -  "__final is a GNU extension, consider using C++11 final">,
> -  InGroup;
> -
> -def warn_shadow_field :
> -  Warning<"non-static data member %0 of %1 shadows member inherited from "
> -  "type %2">,
> -  InGroup, DefaultIgnore;
> -def note_shadow_field : Note<"declared here">;
> -
> -def err_multiversion_required_in_redecl : Error<
> +  "__final is a GNU extension, consider using C++11 final">,
> +  InGroup;
> +
> +def warn_shadow_field : Warning<
> +  "%select{parameter|non-static data member}3 %0 %select{|of %1 }3shadows "
> +  "member inherited from type %2">, InGroup, DefaultIgnore;
> +def note_shadow_field : Note<"declared here">;
> +
> +def err_multiversion_required_in_redecl : Error<
>   "function declaration is missing %select{'target'|'cpu_specific' or "
>   "'cpu_dispatch'}0 attribute in a multiversioned function">;
> def note_multiversioning_caused_here : Note<
> 
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=346041=346040=346041=diff
> ==
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Fri Nov  2 14:04:44 2018
> @@ -10588,7 +10588,8 @@ private:
>   /// Check if there is a field shadowing.
>   void CheckShadowInheritedFields(const SourceLocation ,
>   DeclarationName FieldName,
> -  const CXXRecordDecl *RD);
> +  const CXXRecordDecl *RD,
> +  bool DeclIsField = true);
> 
>   /// Check if the given expression contains 'break' or 'continue'
>   /// statement that produces control flow different from GCC.
> 
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=346041=346040=346041=diff
> ==
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Nov  2 14:04:44 2018
> @@ -12366,6 +12366,13 @@ Decl *Sema::ActOnParamDeclarator(Scope *
> D.setInvalidType(true);
>   }
> }
> +
> +if (LangOpts.CPlusPlus) {
> +  DeclarationNameInfo DNI = GetNameForDeclarator(D);
> +  if (auto *RD = dyn_cast(CurContext))
> +CheckShadowInheritedFields(DNI.getLoc(), DNI.getName(), RD,
> +   /*DeclIsField*/ false);
> +}
>   }
> 
>   // Temporarily put parameter variables in the translation unit, not
> 
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=346041=346040=346041=diff
> ==
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Nov  2 14:04:44 2018
> @@ -2832,13 +2832,14 @@ static const ParsedAttr *getMSPropertyAt
>   return nullptr;
> }
> 
> -// Check if there is a field shadowing.
> -void 

[PATCH] D53280: [analyzer] Emit an error for invalid -analyzer-config inputs

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Ugh. We broke backwards compatibility when an invalid `-analyzer-config` is 
specified but `-analyze` isn't specified. Yes, people are accidentally relying 
on that :/ :/ :/

In this case the driver doesn't know that it needs to add 
`analyzer-config-compatibility-mode=true` (or, even better, outright ignore all 
analyzer options) but the frontend doesn't know that it doesn't need to parse 
them.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53280



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

[PATCH] D55269: [CUDA][OpenMP] Fix nvidia-cuda-toolkit detection on Debian/Ubuntu

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

jdenny added a comment.

In D55269#1319382 , @tra wrote:

> Let's start with fixing OpenMP's cmake files. Once it no longer insists on 
> specifying --cuda-path=/usr, and isUbuntu is in place, what is the remaining 
> failure that you see?


I'm fairly certain I would see no remaining failure then, but it's not clear to 
me how we should convince OpenMP's cmake files to stop doing that.

openmp/libomptarget/cmake/Modules/LibomptargetNVPTXBitcodeLibrary.cmake is the 
file, and there's one occurrence of --cuda-path.

If CUDA_TOOLKIT_ROOT_DIR is specified explicitly to cmake, that --cuda-path is 
necessary, so we cannot just remove it.

If CUDA_TOOLKIT_ROOT_DIR is not specified explicitly, it is computed by cmake.  
So I believe this boils down to getting cmake to find the right CUDA root on 
Debian/Ubuntu.

Do you agree so far?

Would you recommend submitting a patch to cmake's CUDA support?  Or would you 
recommend replicating clang's logic from D40453 
 into openmp's cmake files, overriding cmake's 
own selection of CUDA_TOOLKIT_ROOT_DIR?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55269



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

[PATCH] D54592: [analyzer][CStringChecker] evaluate explicit_bzero

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

Szelethus added a comment.

I hope you don't mind me changing the revision title -- many of us are 
automatically subscribed to revisions with `analyzer` in the title, that also 
helps with getting feedback sooner :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D54592



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

[PATCH] D54592: [CStringChecker] evaluate explicit_bzero

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

When you're doing something but it isn't working, i encourage you to 
investigate it more pro-actively, or at least add a FIXME so that people didn't 
think that this is the intended behavior.




Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2232
+
+  memsetAux(Mem, Zero, Size, C, State);
+}

This `Zero` is of the wrong type: it is `size_t` but it should be `int`. You'll 
need to make a new one.



Comment at: test/Analysis/string.c:1399
+  bzero(str, 2);
+  clang_analyzer_eval(strlen(str) == 0); // expected-warning{{UNKNOWN}}
+}

I suspect that the reason why this didn't work is that you forgot 
`.addTransition()`. 

Could you also test that `bzero(str + 2, 2);` doesn't turn `strlen(str)` into 
zero?



Comment at: test/Analysis/string.c:1413-1414
+
+  clang_analyzer_eval(strlen(passwd) == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(passwd[0] == '\0'); // expected-warning{{UNKNOWN}}
+}

Probably same `.addTransition()` problem here.


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

https://reviews.llvm.org/D54592



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

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 

[Stephane Moore via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348317: [clang-tidy] Ignore namespaced and C++ member 
functions in google-objc-function… (authored by stephanemoore, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55101?vs=176206=176726#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55101

Files:
  clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.mm


Index: clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.mm
===
--- clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.mm
+++ clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.mm
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+void printSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not
+// using function naming conventions described by Google Objective-C style 
guide
+
+void PrintSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not
+// using function naming conventions described by Google Objective-C style 
guide
+
+void ABCBad_Name() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not
+// using function naming conventions described by Google Objective-C style 
guide
+
+namespace {
+
+int foo() { return 0; }
+
+}
+
+namespace bar {
+
+int convert() { return 0; }
+
+}
+
+class Baz {
+public:
+  int value() { return 0; }
+};
Index: clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
@@ -98,8 +98,9 @@
   // main.
   Finder->addMatcher(
   functionDecl(
-  unless(isExpansionInSystemHeader()),
-  unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
+  unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
+   hasAncestor(namespaceDecl()), isMain(),
+   matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))
   .bind("function"),


Index: clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.mm
===
--- clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.mm
+++ clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.mm
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+void printSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not
+// using function naming conventions described by Google Objective-C style guide
+
+void PrintSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not
+// using function naming conventions described by Google Objective-C style guide
+
+void ABCBad_Name() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not
+// using function naming conventions described by Google Objective-C style guide
+
+namespace {
+
+int foo() { return 0; }
+
+}
+
+namespace bar {
+
+int convert() { return 0; }
+
+}
+
+class Baz {
+public:
+  int value() { return 0; }
+};
Index: clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
@@ -98,8 +98,9 @@
   // main.
   Finder->addMatcher(
   functionDecl(
-  unless(isExpansionInSystemHeader()),
-  unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
+  unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
+   hasAncestor(namespaceDecl()), isMain(),
+   matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))
   .bind("function"),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Chatted offline with Ben and confirmed that he's okay with proceeding.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55101



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

[PATCH] D54592: [CStringChecker] evaluate explicit_bzero

[David CARLIER via Phabricator via cfe-commits]

devnexen updated this revision to Diff 176722.

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

https://reviews.llvm.org/D54592

Files:
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  test/Analysis/string.c

Index: test/Analysis/string.c
===
--- test/Analysis/string.c
+++ test/Analysis/string.c
@@ -1184,11 +1184,14 @@
 }
 
 //===--===
-// memset()
+// memset() / explicit_bzero() / bzero()
 //===--===
 
 void *memset(void *dest, int ch, size_t count);
 
+void bzero(void *dst, size_t count);
+void explicit_bzero(void *dest, size_t count);
+
 void *malloc(size_t size);
 void free(void *);
 
@@ -1383,6 +1386,45 @@
   clang_analyzer_eval(array[4] == 0); // expected-warning{{TRUE}}
 }
 
+void bzero1_null() {
+  char *a = NULL;
+
+  bzero(a, 10); // expected-warning{{Null pointer argument in call to memory clearance function}}
+}
+
+void bzero2_char_array_null() {
+  char str[] = "abcd";
+  clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+  bzero(str, 2);
+  clang_analyzer_eval(strlen(str) == 0); // expected-warning{{UNKNOWN}}
+}
+
+void explicit_bzero1_null() {
+  char *a = NULL;
+
+  explicit_bzero(a, 10); // expected-warning{{Null pointer argument in call to memory clearance function}}
+}
+
+void explicit_bzero2_clear_mypassword() {
+  char passwd[7] = "passwd";
+
+  explicit_bzero(passwd, sizeof(passwd)); // no-warning
+
+  clang_analyzer_eval(strlen(passwd) == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(passwd[0] == '\0'); // expected-warning{{UNKNOWN}}
+}
+
+#ifdef SUPPRESS_OUT_OF_BOUND
+void explicit_bzero3_out_ofbound() {
+  char *privkey = (char *)malloc(6);
+  const char newprivkey[10] = "mysafekey";
+
+  strcpy(privkey, "random");
+  explicit_bzero(privkey, sizeof(newprivkey));
+  clang_analyzer_eval(privkey[0] == '\0'); // expected-warning{{UNKNOWN}}
+  free(privkey);
+}
+#endif
 //===--===
 // FIXMEs
 //===--===
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -124,6 +124,7 @@
   void evalStdCopyBackward(CheckerContext , const CallExpr *CE) const;
   void evalStdCopyCommon(CheckerContext , const CallExpr *CE) const;
   void evalMemset(CheckerContext , const CallExpr *CE) const;
+  void evalExplicitBzero(CheckerContext , const CallExpr *CE) const;
 
   // Utility methods
   std::pair
@@ -158,7 +159,7 @@
   static bool SummarizeRegion(raw_ostream , ASTContext ,
   const MemRegion *MR);
 
-  static bool memsetAux(const Expr *DstBuffer, const Expr *CharE,
+  static bool memsetAux(const Expr *DstBuffer, SVal CharE,
 const Expr *Size, CheckerContext ,
 ProgramStateRef );
 
@@ -1005,11 +1006,10 @@
   }
 }
 
-bool CStringChecker::memsetAux(const Expr *DstBuffer, const Expr *CharE,
+bool CStringChecker::memsetAux(const Expr *DstBuffer, SVal CharVal,
const Expr *Size, CheckerContext ,
ProgramStateRef ) {
   SVal MemVal = C.getSVal(DstBuffer);
-  SVal CharVal = C.getSVal(CharE);
   SVal SizeVal = C.getSVal(Size);
   const MemRegion *MR = MemVal.getAsRegion();
   if (!MR)
@@ -2184,13 +2184,54 @@
   // According to the values of the arguments, bind the value of the second
   // argument to the destination buffer and set string length, or just
   // invalidate the destination buffer.
-  if (!memsetAux(Mem, CharE, Size, C, State))
+  if (!memsetAux(Mem, C.getSVal(CharE), Size, C, State))
 return;
 
   State = State->BindExpr(CE, LCtx, MemVal);
   C.addTransition(State);
 }
 
+void CStringChecker::evalExplicitBzero(CheckerContext , const CallExpr *CE) const {
+  if (CE->getNumArgs() != 2)
+return;
+
+  CurrentFunctionDescription = "memory clearance function";
+
+  const Expr *Mem = CE->getArg(0);
+  const Expr *Size = CE->getArg(1);
+  SVal Zero = C.getSValBuilder().makeZeroVal(Size->getType());
+
+  ProgramStateRef State = C.getState();
+  
+  // See if the size argument is zero.
+  SVal SizeVal = C.getSVal(Size);
+  QualType SizeTy = Size->getType();
+
+  ProgramStateRef StateZeroSize, StateNonZeroSize;
+  std::tie(StateZeroSize, StateNonZeroSize) =
+assumeZero(C, State, SizeVal, SizeTy);
+
+  // If the size is zero, there won't be any actual memory access,
+  // In this case we just return.
+  if (StateZeroSize && !StateNonZeroSize)
+return;
+
+  // Get the value of the memory area.
+  SVal MemVal = C.getSVal(Mem);
+
+  // Ensure the memory area is not null.
+  // If it is NULL there will be a NULL 

[PATCH] D55269: [CUDA][OpenMP] Fix nvidia-cuda-toolkit detection on Debian/Ubuntu

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D55269#1319319 , @jdenny wrote:

> My real goal is to get clang and openmp working out of the box on Ubuntu.  
> Treating --cuda-path=/usr as a special case was just a way to get there.  
> It's odd apparently because nvidia-cuda-toolkit is odd.


It does not address the issue in principle -- if we add tweaks do deal with 
Ubuntu oddities, why shouldn't we also add tweaks for any other linux distro?
If you want to solve the issue in general you need to be able to tell where to 
find each component of CUDA SDK needed by clang. Once you have that 
flexibility, it will open a lot of possibilities to specify one of them 
incorrectly and fail in some strange way nobody has seen before. I don't think 
it's worth the effort.

I personally prefer to keep things as simple as possible. Clang expects to see 
a monolithic CUDA SDK installation, specified, if necessary, with one option.
Dealing with distro-specific way SUDA SDK may be split there is up to the 
distro maintainers. If they want Clang compilation work with their variant of 
CUDA installation, they can always create a shim the way Debian did.

As for cmake, it currently does not support clang as the CUDA compiler. 
Whatever detects CUDA install path apparently does not do it correctly on 
Ubuntu (it's the same problem of having CUDA SDK bits all over the place) and 
that's something that needs to be fixed there.

> I don't have a separate project using cmake.  The cmake files I'm referring 
> to are clang's.  I'm just trying to make it easy to build clang and openmp 
> and call clang on the command line under Ubuntu.
>  An easier workaround is to specify CUDA_TOOLKIT_ROOT_DIR when building llvm,

That would work, too.

> but my goal is make building on Ubuntu work without special configuration.  
> D40453  was my hint that people already 
> agreed that's a worthwhile pursuit.

And I believe that we did convince Debian that it's up to them to arrange their 
packages in a way that works with clang.
Granted, closer examination of the shim they've added shows that the shim is 
incomplete, but it's the right way to solve this problem, IMO.

> I'm not adamant that handling --cuda-path=/usr is the right solution.  But 
> just adding IsUbuntu() is not a full solution, so I'm looking for advice on 
> how to proceed.

Let's start with fixing OpenMP's cmake files. Once it no longer insists on 
specifying --cuda-path=/usr, and isUbuntu is in place, what is the remaining 
failure that you see?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55269



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

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[Leonard Chan via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaExpr.cpp:1304
+RHSTy = ResultTy;
+  }
+

rjmccall wrote:
> leonardchan wrote:
> > rjmccall wrote:
> > > leonardchan wrote:
> > > > rjmccall wrote:
> > > > > ebevhan wrote:
> > > > > > rjmccall wrote:
> > > > > > > leonardchan wrote:
> > > > > > > > rjmccall wrote:
> > > > > > > > > ebevhan wrote:
> > > > > > > > > > rjmccall wrote:
> > > > > > > > > > > Hmm.  So adding a signed integer to an unsigned 
> > > > > > > > > > > fixed-point type always converts the integer to unsigned? 
> > > > > > > > > > >  That's a little weird, but only because the fixed-point 
> > > > > > > > > > > rule seems to be the other way.  Anyway, I assume it's 
> > > > > > > > > > > not a bug in the spec.
> > > > > > > > > > `handleFixedPointConversion` only calculates the result 
> > > > > > > > > > type of the expression, not the calculation type. The final 
> > > > > > > > > > result type of the operation will be the unsigned 
> > > > > > > > > > fixed-point type, but the calculation itself will be done 
> > > > > > > > > > in a signed type with enough precision to represent both 
> > > > > > > > > > the signed integer and the unsigned fixed-point type. 
> > > > > > > > > > 
> > > > > > > > > > Though, if the result ends up being negative, the final 
> > > > > > > > > > result is undefined unless the type is saturating. I don't 
> > > > > > > > > > think there is a test for the saturating case (signed 
> > > > > > > > > > integer + unsigned saturating fixed-point) in the 
> > > > > > > > > > SaturatedAddition tests.
> > > > > > > > > > `handleFixedPointConversion` only calculates the result 
> > > > > > > > > > type of the expression, not the calculation type.
> > > > > > > > > 
> > > > > > > > > Right, I understand that, but the result type of the 
> > > > > > > > > expression obviously impacts the expressive range of the 
> > > > > > > > > result, since you can end up with a negative value.
> > > > > > > > > 
> > > > > > > > > Hmm.  With that said, if the general principle is to perform 
> > > > > > > > > the operation with full precision on the original operand 
> > > > > > > > > values, why are unsigned fixed-point operands coerced to 
> > > > > > > > > their corresponding signed types *before* the operation?  
> > > > > > > > > This is precision-limiting if the unsigned representation 
> > > > > > > > > doesn't use a padding bit.  That seems like a bug in the spec.
> > > > > > > > > Hmm. With that said, if the general principle is to perform 
> > > > > > > > > the operation with full precision on the original operand 
> > > > > > > > > values, why are unsigned fixed-point operands coerced to 
> > > > > > > > > their corresponding signed types *before* the operation? This 
> > > > > > > > > is precision-limiting if the unsigned representation doesn't 
> > > > > > > > > use a padding bit. That seems like a bug in the spec.
> > > > > > > > 
> > > > > > > > Possibly. When the standard mentions converting from signed to 
> > > > > > > > unsigned fixed point, the only requirement involved is 
> > > > > > > > conservation of magnitude (the number of integral bits 
> > > > > > > > excluding the sign)
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > when signed and unsigned fixed-point types are mixed, the 
> > > > > > > > unsigned type is converted to the corresponding signed type, 
> > > > > > > > and this should go without loss of magnitude
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > This is just speculation, but I'm under the impression that not 
> > > > > > > > as much "attention" was given for unsigned types as for signed 
> > > > > > > > types since "`In the embedded processor world, support for 
> > > > > > > > unsigned fixed-point data types is rare; normally only signed 
> > > > > > > > fixed-point data types are supported`", but was kept anyway 
> > > > > > > > since unsigned types are used a lot.
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > However, to disallow unsigned fixed-point arithmetic from 
> > > > > > > > programming languages (in general, and from C in particular) 
> > > > > > > > based on this observation, seems overly restrictive.
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > I also imagine that if the programmer needs more precision, the 
> > > > > > > > correct approach would be to cast up to a type with a higher 
> > > > > > > > scale. The standard seems to make an effort to expose as much 
> > > > > > > > in regards to the underlying fixed point types as possible:
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > it should be possible to write fixed-point algorithms that are 
> > > > > > > > independent of the actual fixed-point hardware support. This 
> > > > > > > > implies that a programmer (or a running program) should have 
> > > > > > > > access to all parameters that define the behavior of the 
> > > > > > > > 

[PATCH] D55269: [CUDA][OpenMP] Fix nvidia-cuda-toolkit detection on Debian/Ubuntu

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

jdenny added a comment.

In D55269#1319319 , @jdenny wrote:

> > I do not think that changing clang to work around an issue in cmake files 
> > of one project is something we want to do.
>
> I don't have a separate project using cmake.  The cmake files I'm referring 
> to are clang's.


... and opemp's.  Is that the one project you meant?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55269



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

[PATCH] D55269: [CUDA][OpenMP] Fix nvidia-cuda-toolkit detection on Debian/Ubuntu

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

jdenny added a comment.

In D55269#1319252 , @tra wrote:

> It appears that what you're trying to do is to add "/usr/lib/cuda" on Ubuntu 
> and Debian when --cuda-path=/usr is specified.
>  This is a rather odd thing to do.


My real goal is to get clang and openmp working out of the box on Ubuntu.  
Treating --cuda-path=/usr as a special case was just a way to get there.  It's 
odd apparently because nvidia-cuda-toolkit is odd.

> In the end only one of those paths will be in effect and that's the path that 
> should be specified via --cuda-path. The fact that you want to add 
> /usr/cuda/lib in this case suggests that /usr is the wrong path and 
> /usr/lib/cuda is the correct one. It sounds like you need to change your 
> build system and tell clang the correct path.
> 
> I do not think that changing clang to work around an issue in cmake files of 
> one project is something we want to do.

I don't have a separate project using cmake.  The cmake files I'm referring to 
are clang's.  I'm just trying to make it easy to build clang and openmp and 
call clang on the command line under Ubuntu.

> There is also an easy workaround. Just download CUDA SDK, install it into a 
> local directory and point your build system there. This will work on any 
> linux distribution.

An easier workaround is to specify CUDA_TOOLKIT_ROOT_DIR when building llvm, 
but my goal is make building on Ubuntu work without special configuration.  
D40453  was my hint that people already agreed 
that's a worthwhile pursuit.

I'm not adamant that handling --cuda-path=/usr is the right solution.  But just 
adding IsUbuntu() is not a full solution, so I'm looking for advice on how to 
proceed.

Thanks for your responses.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55269



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

[PATCH] D55066: [ASan] Minor documentation fix: clarify static linking limitation.

[Max Moroz via Phabricator via cfe-commits]

Dor1s added a comment.

Thanks @eugenis for explaining the issue to me over chat. I've updated the CL 
and the description. I can abandon it though, if you find it useless.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55066



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

[PATCH] D55270: [Sema] Further improvements to to static_assert diagnostics.

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

Quuxplusone added inline comments.



Comment at: lib/Sema/SemaTemplate.cpp:3062
+public:
+  FailedBooleanConditionPrinterHelper(const PrintingPolicy )
+  : Policy(Policy) {}

`explicit`



Comment at: lib/Sema/SemaTemplate.cpp:3065
+
+  ~FailedBooleanConditionPrinterHelper() override {}
+

aaron.ballman wrote:
> Is this definition necessary?
Nit: I don't know if it is LLVM style to provide explicitly written-out 
overrides for all virtual destructors. In my own code, if a destructor would be 
redundant, I wouldn't write it.



Comment at: lib/Sema/SemaTemplate.cpp:3089
+
+} // namespace
 

`} // end anonymous namespace`



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

Please also add a test case for the `is_const_v` inline-variable-template 
version.
```
template
inline constexpr bool is_const_v = is_const::value;
static_assert(is_const_v, "message");  // if this test case 
was missing from the previous patch
static_assert(!is_const_v, "message");  // exercise the 
same codepath for this new feature
```

Also, does using the PrinterHelper mean that you get a bunch of other cases for 
free? Like, does this work now too?
```
static_assert(is_const::value == false, "message");
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D55270



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

[PATCH] D55066: [ASan] Minor documentation fix: remove static linking limitation.

[Max Moroz via Phabricator via cfe-commits]

Dor1s updated this revision to Diff 176712.
Dor1s added a comment.

Restore the message with a couple clarifying words.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55066

Files:
  docs/AddressSanitizer.rst


Index: docs/AddressSanitizer.rst
===
--- docs/AddressSanitizer.rst
+++ docs/AddressSanitizer.rst
@@ -265,7 +265,7 @@
 * On 64-bit platforms AddressSanitizer maps (but not reserves) 16+ Terabytes of
   virtual address space. This means that tools like ``ulimit`` may not work as
   usually expected.
-* Static linking is not supported.
+* Static linking of executables is not supported.
 
 Supported Platforms
 ===


Index: docs/AddressSanitizer.rst
===
--- docs/AddressSanitizer.rst
+++ docs/AddressSanitizer.rst
@@ -265,7 +265,7 @@
 * On 64-bit platforms AddressSanitizer maps (but not reserves) 16+ Terabytes of
   virtual address space. This means that tools like ``ulimit`` may not work as
   usually expected.
-* Static linking is not supported.
+* Static linking of executables is not supported.
 
 Supported Platforms
 ===
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r348313 - Fix crash if an in-class explicit function specialization has explicit

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Dec  4 14:26:32 2018
New Revision: 348313

URL: http://llvm.org/viewvc/llvm-project?rev=348313=rev
Log:
Fix crash if an in-class explicit function specialization has explicit
template arguments referring to template paramaeters.

Added:
cfe/trunk/test/SemaTemplate/member-specialization.cpp
Modified:
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=348313=348312=348313=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Tue Dec  4 14:26:32 2018
@@ -2702,26 +2702,28 @@ Decl *TemplateDeclInstantiator::VisitUsi
 }
 
 Decl *TemplateDeclInstantiator::VisitClassScopeFunctionSpecializationDecl(
- ClassScopeFunctionSpecializationDecl 
*Decl) {
+ClassScopeFunctionSpecializationDecl *Decl) {
   CXXMethodDecl *OldFD = Decl->getSpecialization();
   CXXMethodDecl *NewFD =
 cast_or_null(VisitCXXMethodDecl(OldFD, nullptr, true));
   if (!NewFD)
 return nullptr;
 
-  LookupResult Previous(SemaRef, NewFD->getNameInfo(), 
Sema::LookupOrdinaryName,
-Sema::ForExternalRedeclaration);
-
-  TemplateArgumentListInfo TemplateArgs;
-  TemplateArgumentListInfo *TemplateArgsPtr = nullptr;
+  TemplateArgumentListInfo ExplicitTemplateArgs;
+  TemplateArgumentListInfo *ExplicitTemplateArgsPtr = nullptr;
   if (Decl->hasExplicitTemplateArgs()) {
-TemplateArgs = Decl->templateArgs();
-TemplateArgsPtr = 
+if (SemaRef.Subst(Decl->templateArgs().getArgumentArray(),
+  Decl->templateArgs().size(), ExplicitTemplateArgs,
+  TemplateArgs))
+  return nullptr;
+ExplicitTemplateArgsPtr = 
   }
 
+  LookupResult Previous(SemaRef, NewFD->getNameInfo(), 
Sema::LookupOrdinaryName,
+Sema::ForExternalRedeclaration);
   SemaRef.LookupQualifiedName(Previous, SemaRef.CurContext);
-  if (SemaRef.CheckFunctionTemplateSpecialization(NewFD, TemplateArgsPtr,
-  Previous)) {
+  if (SemaRef.CheckFunctionTemplateSpecialization(
+  NewFD, ExplicitTemplateArgsPtr, Previous)) {
 NewFD->setInvalidDecl();
 return NewFD;
   }

Added: cfe/trunk/test/SemaTemplate/member-specialization.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/member-specialization.cpp?rev=348313=auto
==
--- cfe/trunk/test/SemaTemplate/member-specialization.cpp (added)
+++ cfe/trunk/test/SemaTemplate/member-specialization.cpp Tue Dec  4 14:26:32 
2018
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -std=c++17 -verify %s
+// expected-no-diagnostics
+
+template struct X {
+  template const V () { return V::error; }
+  template<> const U () { return u; }
+  U u;
+};
+int f(X x) {
+  return x.as();
+}


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

[PATCH] D55270: [Sema] Further improvements to to static_assert diagnostics.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaTemplate.cpp:3065
+
+  ~FailedBooleanConditionPrinterHelper() override {}
+

Is this definition necessary?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55270



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

[PATCH] D55067: [HIP] Fix offset of kernel argument for AMDGPU target

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D55067#1319047 , @arsenm wrote:

> I don't understand how the alignment of the IR type isn't meaningful or 
> stable. The DataLayout gives us the concept of an ABI alignment, but you seem 
> to be saying this is meaningless (in which case why do we have it?)


LLVM has stable rules for computing the alignment of a particular LLVM struct 
type.  Clang IRGen does not have stable rules for turning a Clang struct type 
into an LLVM struct type.  Clang always reserves the right to translate a 
particular source-language struct type into, say, `{ [8 x i8] }` instead of `{ 
i32, i32 }`.  This is unlikely for sufficiently simple types just because it's 
nice for debugging and for the compactness of the generated code if the 
generated struct type is reasonably similar to the original type, but there are 
complex cases where the translation is not totally obvious, and we don't want 
to make promises.  Because the translation to LLVM IR type is not stable, the 
choice of alignment for the LLVM IR type cannot be guaranteed either.

> I think we have different understandings of what an ABI means in this case. 
> The "call" to a kernel isn't made in a vacuum unlike a normal function. For 
> OpenCL at least, the backend produces metadata in the object file telling the 
> explicit offsets of the individual arguments, which we today compute from the 
> IR argument list. The actual caller isn't using the types to decide where to 
> place arguments, the binary is explicitly telling it where. For some reason 
> CUDA seems to be doing something different here which might be part of my 
> confusion?

If you don't guarantee stability across recompilations for the size and layout 
of this buffer, and neither callers nor callees make assumptions about the 
alignment of pointers within it, then you might indeed not have any ABI 
requirements.  But in that case you might as well just use a packed structure 
so that you can minimize the number of bytes you have to transfer.

>>> The alternative I would like is to stop using the IR argument list at all 
>>> for kernels. All arguments would be accessed from offsets from an intrinsic 
>>> call
>> 
>> That seems like a very reasonable alternative, or you could have the 
>> function take a single pointer argument and do all accesses relative to 
>> that.  I'll leave that up to you.
> 
> We would need to make up some other way to track the individual arguments, 
> which I haven't come up with a great solution for (other than keeping them 
> there without uses, which seems pretty awkward). As an optimization we 
> currently do lower the kernel argument uses to look like this, but we compute 
> the offsets from the IR arguments.

Okay.  It seems to me like there should be exactly one place responsible for 
computing the layout of this buffer.  Like, if you really don't care about 
alignment, you could pack it; or if you do care about alignment but don't need 
stability, you could reorder the fields to minimize alignment padding; or 
really anything along those lines.


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

https://reviews.llvm.org/D55067



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

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: test/Sema/format-strings-bitfield-promotion.c:1
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
+

ebevhan wrote:
> aaron.ballman wrote:
> > ebevhan wrote:
> > > ebevhan wrote:
> > > > aaron.ballman wrote:
> > > > > aaron.ballman wrote:
> > > > > > Running your test through GCC looks like the behavior matches here 
> > > > > > for C; can you also add a C++ test that demonstrates the behavior 
> > > > > > does not change?
> > > > > > 
> > > > > > https://godbolt.org/z/zRYDMG
> > > > > Strangely, the above godbolt link dropped the output windows, here's 
> > > > > a different link that shows the behavioral differences between C and 
> > > > > C++ mode in GCC: https://godbolt.org/z/R3zRHe
> > > > Hmm, I'll have a look at this.
> > > That gcc godbolt is a bit odd. The type of the bitfield expression in the 
> > > C++ example is `long` and not `int`, but in Clang, it's clearly being 
> > > converted. If I change the example a bit, we get this warning:
> > > ```
> > > :11:12: warning: format '%d' expects argument of type 'int', but 
> > > argument 2 has type 'long int' [-Wformat=]
> > >11 |   printf("%d", bf.a); // expected-warning {{format specifies type 
> > > 'long' but the argument has type 'int'}}
> > >   |   ~^   
> > >   ||  |
> > >   |intlong int
> > > ```
> > > But in Clang, we get a cast to `int`:
> > > ```
> > > | `-ImplicitCastExpr 0xd190748  'int' 
> > > |   `-ImplicitCastExpr 0xd190730  'long' 
> > > 
> > > | `-MemberExpr 0xd190618  'long' lvalue bitfield 
> > > .a 0xd18f790
> > > |   `-DeclRefExpr 0xd1905f8  'struct 
> > > bitfields':'bitfields' lvalue Var 0xd18fa18 'bf' 'struct 
> > > bitfields':'bitfields'
> > > ```
> > > 
> > > So gcc and Clang are doing things differently here.
> > > 
> > > The code in `isPromotableBitField` says:
> > > ```
> > >   // FIXME: C does not permit promotion of a 'long : 3' bitfield to int.
> > >   //We perform that promotion here to match GCC and C++.
> > > ```
> > > but clearly gcc isn't doing this in the C++ case. The comments also 
> > > mention some things about gcc bugs that Clang does not follow, but that's 
> > > in reference to a C DR.
> > C++ disallows the rank conversion from int to long as well. [conv.prom]p1 
> > does not apply because `long int` has a higher rank than `int`, but 
> > [conv.prom]p5 allows the promotion if the range of values is identical 
> > between the two types.
> > 
> > C makes this UB in several ways -- you can't have a bit-field whose type is 
> > something other than int, unsigned int, or _Bool (6.7.2.1p5) or promoting 
> > from types other than those (6.3.1.1p2), but otherwise matches the C++ 
> > behavior in terms of promotion (including the rank conversion).
> > 
> > You may have to dig further into what Clang is doing, but I would guess 
> > that the diagnostics should be triggered in both C and C++ similarly.
> > 
> > Ultimately, I'd like to see tests for cases where `sizeof(int) == 
> > sizeof(long)`, `sizeof(int) != sizeof(long)`, and variants for C and C++ of 
> > each.
> I'm not sure the warning should trigger in C++; the behavior is correct 
> there. The expression in those cases should be of type `long`, not `int`. The 
> bitfield promotions in C++ say that values _can_ be promoted if the value 
> fits in `int`, but the rules in C say that the value _is_ promoted.
> 
> The strange promotion behavior only occurs in C because of the issue with 
> bitfields larger than int. It's not really permitted according to the 
> standard, but it's supported anyway to match C++. Though, it ends up not 
> matching C++ due to these promotion differences.
> 
> I'll add tests for the different int and long sizes, though the only case 
> where it would make a difference would be if int was larger than 32 bits, 
> which it isn't on any target.
> The bitfield promotions in C++ say that values _can_ be promoted if the value 
> fits in int, but the rules in C say that the value _is_ promoted.

Ahhh, that explains the differences (my eyes glossed over that in the standards 
text), thank you!


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

https://reviews.llvm.org/D51211



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

[PATCH] D55269: [CUDA][OpenMP] Fix nvidia-cuda-toolkit detection on Debian/Ubuntu

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

It appears that what you're trying to do is to add "/usr/lib/cuda" on Ubuntu 
and Debian when --cuda-path=/usr is specified.
This is a rather odd thing to do. In the end only one of those paths will be in 
effect and that's the path that should be specified via --cuda-path. The fact 
that you want to add /usr/cuda/lib in this case suggests that /usr is the wrong 
path and /usr/lib/cuda is the correct one. It sounds like you need to change 
your build system and tell clang the correct path.

I do not think that changing clang to work around an issue in cmake files of 
one project is something we want to do.

There is also an easy workaround. Just download CUDA SDK, install it into a 
local directory and point your build system there. This will work on any linux 
distribution.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55269



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

[PATCH] D55189: Extract TextNodeDumper class

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: lib/AST/ASTDumper.cpp:90
 // Utilities
-void dumpPointer(const void *Ptr);
-void dumpSourceRange(SourceRange R);
-void dumpLocation(SourceLocation Loc);
-void dumpBareType(QualType T, bool Desugar = true);
-void dumpType(QualType T);
+void dumpType(QualType T) { NodeDumper.dumpType(T); }
 void dumpTypeAsChild(QualType T);

Another nice cleanup for later would be to replace these call sites with the 
`NodeDumper.foo()` version. Same for `dumpBareDeclRef()` below.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55189



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

[PATCH] D55188: Extract TextChildDumper class

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a commenting nit.




Comment at: include/clang/AST/ASTDumperUtils.h:112
+public:
+  /// Dump a child of the current node.
+  template  void addChild(Fn doAddChild) {

I really like the rename, but can you update this comment to not say "dump" as 
that implies to me that the writing happens immediately. It might also be a 
kindness to add the expected function signature of `Fn` to the comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55188



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

[PATCH] D54866: Cleanups in IdentifierInfo following the removal of PTH

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

Can this go in now that D54547  has been 
committed ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54866



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

[PATCH] D55257: Inline handling of DependentSizedArrayType

[Aaron Ballman via Phabricator via cfe-commits]

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

In D55257#1319016 , @steveire wrote:

> In D55257#1318769 , @aaron.ballman 
> wrote:
>
> > In D55257#1318376 , @steveire 
> > wrote:
> >
> > > In D55257#1318328 , 
> > > @aaron.ballman wrote:
> > >
> > > > > It is necessary to perform all printing before any traversal to child 
> > > > > nodes.
> > > >
> > > > This piqued my interest -- is `VisitFunctionDecl()` then incorrect 
> > > > because it streams output, then dumps parameter children, then dumps 
> > > > more output, then dumps override children? Or do you mean "don't 
> > > > interleave `VisitFoo()` calls with streaming output"?
> > >
> > >
> > > Can you relate your question to https://reviews.llvm.org/D55083 ?
> >
> >
> > Ah, I was looking at code before having fetched those changes, so perhaps 
> > my example is poor. Mostly, I'm wondering what you meant by "traversal to 
> > child nodes" -- do you mean:
> >
> > 1. it's bad to output to the stream, then dumpChild(), then output to the 
> > stream again
> > 2. it's bad to output to the stream, then VisitFoo(), then output to the 
> > stream again
> > 3. both #1 and #2
> > 4. neither #1 nor #2
> >
> >   (as in: when I'm doing a code review a few months from now, what should I 
> > be watching out for in this scenario?)
>
>
> I don't think 'bad' is the right phrasing, but mostly your #1 is correct. 
> VisitFoo() methods often call dumpChild() and even if one doesn't today, it 
> can.
>
> The reason to re-order is that it enables the separation of traversal from 
> outputting. See the split for comments in https://reviews.llvm.org/D55190 and 
> see that `dumpComment` there calls `NodeDumper.Visit`.
>
> When the refactoring is finished, the traversal class will not have a output 
> stream member, so it won't be able to stream to output, so there is nothing 
> you need to keep in mind in 6 months.


Fantastic, thank you for the clarification; LGTM!


Repository:
  rC Clang

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

https://reviews.llvm.org/D55257



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

r348309 - Adding tests for -ast-dump; NFC.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Dec  4 13:50:08 2018
New Revision: 348309

URL: http://llvm.org/viewvc/llvm-project?rev=348309=rev
Log:
Adding tests for -ast-dump; NFC.

This adds tests for the definition data of C++ record objects as well as 
special member functions.

Added:
cfe/trunk/test/AST/ast-dump-record-definition-data.cpp
cfe/trunk/test/AST/ast-dump-special-member-functions.cpp

Added: cfe/trunk/test/AST/ast-dump-record-definition-data.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-record-definition-data.cpp?rev=348309=auto
==
--- cfe/trunk/test/AST/ast-dump-record-definition-data.cpp (added)
+++ cfe/trunk/test/AST/ast-dump-record-definition-data.cpp Tue Dec  4 13:50:08 
2018
@@ -0,0 +1,190 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++17 -ast-dump %s | 
FileCheck -strict-whitespace %s
+
+void f() {
+  auto IsNotGenericLambda = [](){};
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  col:29 implicit class definition
+  // CHECK-NOT: DefinitionData {{.*}}generic{{.*}}
+  // CHECK-NEXT: DefinitionData {{.*}}lambda{{.*}}
+  auto IsGenericLambda = [](auto){};
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  col:26 implicit class definition
+  // CHECK-NEXT: DefinitionData {{.*}}generic{{.*}}lambda{{.*}}
+}
+
+struct CanPassInRegisters {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct CanPassInRegisters definition
+  // CHECK-NEXT: DefinitionData {{.*}}pass_in_registers{{.*}}
+  CanPassInRegisters(const CanPassInRegisters&) = default;
+};
+
+struct CantPassInRegisters {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct CantPassInRegisters definition
+  // CHECK-NOT: DefinitionData {{.*}}pass_in_registers{{.*}}
+  CantPassInRegisters(const CantPassInRegisters&) = delete;
+};
+
+struct IsEmpty {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsEmpty definition
+  // CHECK-NEXT: DefinitionData {{.*}}empty{{.*}}
+};
+
+struct IsNotEmpty {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsNotEmpty definition
+  // CHECK-NOT: DefinitionData {{.*}}empty{{.*}}
+  int a;
+};
+
+struct IsAggregate {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsAggregate definition
+  // CHECK-NEXT: DefinitionData {{.*}}aggregate{{.*}}
+  int a;
+};
+
+struct IsNotAggregate {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsNotAggregate definition
+  // CHECK-NOT: DefinitionData {{.*}}aggregate{{.*}}
+private:
+  int a;
+};
+
+struct IsStandardLayout {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsStandardLayout definition
+  // CHECK-NEXT: DefinitionData {{.*}}standard_layout{{.*}}
+  void f();
+};
+
+struct IsNotStandardLayout {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsNotStandardLayout definition
+  // CHECK-NOT: DefinitionData {{.*}}standard_layout{{.*}}
+  virtual void f();
+};
+
+struct IsTriviallyCopyable {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsTriviallyCopyable definition
+  // CHECK-NEXT: DefinitionData {{.*}}trivially_copyable{{.*}}
+};
+
+struct IsNotTriviallyCopyable {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsNotTriviallyCopyable definition
+  // CHECK-NOT: DefinitionData {{.*}}trivially_copyable{{.*}}
+  IsNotTriviallyCopyable(const IsNotTriviallyCopyable&) {}
+};
+
+struct IsPOD {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsPOD definition
+  // CHECK-NEXT: DefinitionData {{.*}}pod{{.*}}
+  int a;
+};
+
+struct IsNotPOD {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsNotPOD definition
+  // CHECK-NOT: DefinitionData {{.*}}pod{{.*}}
+  int 
+};
+
+struct IsTrivial {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsTrivial definition
+  // CHECK-NEXT: DefinitionData {{.*}}trivial {{.*}}
+  IsTrivial() = default;
+};
+
+struct IsNotTrivial {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsNotTrivial definition
+  // CHECK-NOT: DefinitionData {{.*}}trivial {{.*}}
+  IsNotTrivial() {}
+};
+
+struct IsPolymorphic {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsPolymorphic definition
+  // CHECK-NEXT: DefinitionData {{.*}}polymorphic{{.*}}
+  virtual void f();
+};
+
+struct IsNotPolymorphic {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsNotPolymorphic definition
+  // CHECK-NOT: DefinitionData {{.*}}polymorphic{{.*}}
+  void f();
+};
+
+struct IsAbstract {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsAbstract definition
+  // CHECK-NEXT: DefinitionData {{.*}}abstract{{.*}}
+  virtual void f() = 0;
+};
+
+struct IsNotAbstract {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct IsNotAbstract definition
+  // CHECK-NOT: DefinitionData {{.*}}abstract{{.*}}
+  virtual void f();
+};
+
+struct IsLiteral {
+  // CHECK: 

r348308 - Add tests for dumping base classes; NFC.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Dec  4 13:49:24 2018
New Revision: 348308

URL: http://llvm.org/viewvc/llvm-project?rev=348308=rev
Log:
Add tests for dumping base classes; NFC.

Modified:
cfe/trunk/test/AST/ast-dump-records.cpp

Modified: cfe/trunk/test/AST/ast-dump-records.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-records.cpp?rev=348308=348307=348308=diff
==
--- cfe/trunk/test/AST/ast-dump-records.cpp (original)
+++ cfe/trunk/test/AST/ast-dump-records.cpp Tue Dec  4 13:49:24 2018
@@ -237,3 +237,40 @@ union G {
   // CHECK-NEXT: Field 0x{{[^ ]*}} '' 'G::(anonymous struct at 
{{.*}}:[[@LINE-19]]:3)'
   // CHECK-NEXT: Field 0x{{[^ ]*}} 'f' 'int'
 };
+
+struct Base1 {};
+struct Base2 {};
+struct Base3 {};
+
+struct Derived1 : Base1 {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct Derived1 definition
+  // CHECK: public 'Base1'
+};
+
+struct Derived2 : private Base1 {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct Derived2 definition
+  // CHECK: private 'Base1'
+};
+
+struct Derived3 : virtual Base1 {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct Derived3 definition
+  // CHECK: virtual public 'Base1'
+};
+
+struct Derived4 : Base1, virtual Base2, protected Base3 {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct Derived4 definition
+  // CHECK: public 'Base1'
+  // CHECK-NEXT: virtual public 'Base2'
+  // CHECK-NEXT: protected 'Base3'
+};
+
+struct Derived5 : protected virtual Base1 {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct Derived5 definition
+  // CHECK: virtual protected 'Base1'
+};
+
+template 
+struct Derived6 : virtual public Bases... {
+  // CHECK: CXXRecordDecl 0x{{[^ ]*}}  
line:[[@LINE-1]]:8 struct Derived6 definition
+  // CHECK: virtual public 'Bases'...
+};


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

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

Hi Bruno, thanks for working on this!




Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2370
+  "use of this statement in a constexpr %select{function|constructor}0 "
+  "is incompatible with C++ standards before C++20">,
+  InGroup, DefaultIgnore;

I guess this should technically be C++2a



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2431
+  "function try block in constexpr %select{function|constructor}0 is "
+  "incompatible with C++ standards before C++20">,
+  InGroup, DefaultIgnore;

(ditto)



Comment at: lib/Sema/SemaDeclCXX.cpp:1913-1916
+  case Stmt::CXXCatchStmtClass:
+// In case we got a valid constexpr try block, the catch block can be
+// ignored since it will never be evaluated in a constexpr context.
+return true;

I think we still need to check out the catch stmt (even if it'll never be 
evaluated) to make sure that it doesn't have any prohibited statements. i.e., 
we should error here:
```
constexpr int f() { 
  try { return 0; }
  catch (...) {
merp: goto merp;
  }
}
```
But I believe that this patch will accept this.


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

https://reviews.llvm.org/D55097



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

Re: [clang-tools-extra] r348302 - [Documentation] Make options section in Clang-tidy readability-uppercase-literal-suffix consistent with other checks.

[Roman Lebedev via cfe-commits]

On Wed, Dec 5, 2018 at 12:21 AM Eugene Zelenko via cfe-commits
 wrote:
>
> Author: eugenezelenko
> Date: Tue Dec  4 13:19:08 2018
> New Revision: 348302
>
> URL: http://llvm.org/viewvc/llvm-project?rev=348302=rev
> Log:
> [Documentation] Make options section in Clang-tidy 
> readability-uppercase-literal-suffix consistent with other checks.
Thank you!

> Modified:
> 
> clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
>
> Modified: 
> clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst?rev=348302=348301=348302=diff
> ==
> --- 
> clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
>  (original)
> +++ 
> clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
>  Tue Dec  4 13:19:08 2018
> @@ -25,13 +25,21 @@ All valid combinations of suffixes are s
>
>...
>
> -Optionally, a list of the destination suffixes can be provided. When the 
> suffix
> -is found, a case-insensitive lookup in that list is made, and if a 
> replacement
> -is found that is different from the current suffix, then the diagnostic is
> -issued. This allows for fine-grained control of what suffixes to consider and
> -what their replacements should be.
> +Options
> +---
>
> -For example, given a list ``L;uL``:
> +.. option:: NewSuffixes
> +
> +  Optionally, a list of the destination suffixes can be provided. When the
> +  suffix is found, a case-insensitive lookup in that list is made, and if a
> +  replacement is found that is different from the current suffix, then the
> +  diagnostic is issued. This allows for fine-grained control of what 
> suffixes to
> +  consider and what their replacements should be.
> +
> +Example
> +^^^
> +
> +Given a list `L;uL`:
>
>  * ``l`` -> ``L``
>  * ``L`` will be kept as is.
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r348302 - [Documentation] Make options section in Clang-tidy readability-uppercase-literal-suffix consistent with other checks.

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Tue Dec  4 13:19:08 2018
New Revision: 348302

URL: http://llvm.org/viewvc/llvm-project?rev=348302=rev
Log:
[Documentation] Make options section in Clang-tidy 
readability-uppercase-literal-suffix consistent with other checks.

Modified:

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst?rev=348302=348301=348302=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
 (original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
 Tue Dec  4 13:19:08 2018
@@ -25,13 +25,21 @@ All valid combinations of suffixes are s
 
   ...
 
-Optionally, a list of the destination suffixes can be provided. When the suffix
-is found, a case-insensitive lookup in that list is made, and if a replacement
-is found that is different from the current suffix, then the diagnostic is
-issued. This allows for fine-grained control of what suffixes to consider and
-what their replacements should be.
+Options
+---
 
-For example, given a list ``L;uL``:
+.. option:: NewSuffixes
+
+  Optionally, a list of the destination suffixes can be provided. When the
+  suffix is found, a case-insensitive lookup in that list is made, and if a
+  replacement is found that is different from the current suffix, then the
+  diagnostic is issued. This allows for fine-grained control of what suffixes 
to
+  consider and what their replacements should be.
+
+Example
+^^^
+
+Given a list `L;uL`:
 
 * ``l`` -> ``L``
 * ``L`` will be kept as is.


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

[PATCH] D55229: [COFF] Statically link certain runtime library functions

[Mandeep Singh Grang via Phabricator via cfe-commits]

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



Comment at: CodeGenObjC/gnu-init.m:103
+// Make sure we do not have dllimport on the load function
+// CHECK-WIN: declare dso_local void @__objc_load
 

I had to remove dllimport in this and the next unit test. I am not sure of the 
wider implications of this change. Maybe controlling this via a flag (like 
-flto-visibility-public-std) is a better/more localized option?


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

https://reviews.llvm.org/D55229



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

[PATCH] D55229: [COFF] Statically link certain runtime library functions

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang updated this revision to Diff 176693.
mgrang retitled this revision from "[COFF, ARM64] Make 
-flto-visibility-public-std a driver and cc1 flag" to "[COFF] Statically link 
certain runtime library functions".
mgrang edited the summary of this revision.

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

https://reviews.llvm.org/D55229

Files:
  CodeGen/CodeGenModule.cpp
  CodeGen/ms-symbol-linkage.cpp
  CodeGenCXX/runtime-dllstorage.cpp
  CodeGenObjC/gnu-init.m
  CodeGenObjCXX/msabi-stret.mm


Index: CodeGenObjCXX/msabi-stret.mm
===
--- CodeGenObjCXX/msabi-stret.mm
+++ CodeGenObjCXX/msabi-stret.mm
@@ -13,6 +13,5 @@
   return [I m:S()];
 }
 
-// CHECK: declare dllimport void @objc_msgSend_stret(i8*, i8*, ...)
+// CHECK: declare dso_local void @objc_msgSend_stret(i8*, i8*, ...)
 // CHECK-NOT: declare dllimport void @objc_msgSend(i8*, i8*, ...)
-
Index: CodeGenObjC/gnu-init.m
===
--- CodeGenObjC/gnu-init.m
+++ CodeGenObjC/gnu-init.m
@@ -99,6 +99,6 @@
 // Check our load function is in a comdat.
 // CHECK-WIN: define linkonce_odr hidden void @.objcv2_load_function() comdat {
 
-// Make sure we have dllimport on the load function
-// CHECK-WIN: declare dllimport void @__objc_load
+// Make sure we do not have dllimport on the load function
+// CHECK-WIN: declare dso_local void @__objc_load
 
Index: CodeGenCXX/runtime-dllstorage.cpp
===
--- CodeGenCXX/runtime-dllstorage.cpp
+++ CodeGenCXX/runtime-dllstorage.cpp
@@ -108,7 +108,7 @@
 // CHECK-MS-DAG: @_Init_thread_epoch = external thread_local global i32
 // CHECK-MS-DAG: declare dso_local i32 @__tlregdtor(void ()*)
 // CHECK-MS-DAG: declare dso_local i32 @atexit(void ()*)
-// CHECK-MS-DYNAMIC-DAG: declare dllimport {{.*}} void @_CxxThrowException
+// CHECK-MS-DYNAMIC-DAG: declare {{.*}} void @_CxxThrowException
 // CHECK-MS-STATIC-DAG: declare {{.*}} void @_CxxThrowException
 // CHECK-MS-DAG: declare dso_local noalias i8* @"??2@YAPAXI@Z"
 // CHECK-MS-DAG: declare dso_local void @_Init_thread_header(i32*)
Index: CodeGen/ms-symbol-linkage.cpp
===
--- /dev/null
+++ CodeGen/ms-symbol-linkage.cpp
@@ -0,0 +1,20 @@
+// RUN: %clangxx -target aarch64-windows \
+// RUN: -fcxx-exceptions -c -o - %s \
+// RUN: | llvm-objdump -syms - 2>&1 | FileCheck %s
+
+void foo1() { throw 1; }
+// CHECK-LABEL: foo1
+// CHECK-NOT: __imp__CxxThrowException
+
+void bar();
+void foo2() noexcept(true) { bar(); }
+// CHECK-LABEL: foo2
+// CHECK-NOT: __imp___std_terminate
+
+struct A {};
+struct B { virtual void f(); };
+struct C : A, virtual B {};
+struct T {};
+T *foo3() { return dynamic_cast((C *)0); }
+// CHECK-LABEL: foo3
+// CHECK-NOT: __imp___RTDynamicCast
Index: CodeGen/CodeGenModule.cpp
===
--- CodeGen/CodeGenModule.cpp
+++ CodeGen/CodeGenModule.cpp
@@ -2954,7 +2954,8 @@
 
   if (!Local && getTriple().isOSBinFormatCOFF() &&
   !getCodeGenOpts().LTOVisibilityPublicStd &&
-  !getTriple().isWindowsGNUEnvironment()) {
+  !getTriple().isWindowsGNUEnvironment() &&
+  !getTriple().isWindowsMSVCEnvironment()) {
 const FunctionDecl *FD = GetRuntimeFunctionDecl(Context, Name);
 if (!FD || FD->hasAttr()) {
   F->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);


Index: CodeGenObjCXX/msabi-stret.mm
===
--- CodeGenObjCXX/msabi-stret.mm
+++ CodeGenObjCXX/msabi-stret.mm
@@ -13,6 +13,5 @@
   return [I m:S()];
 }
 
-// CHECK: declare dllimport void @objc_msgSend_stret(i8*, i8*, ...)
+// CHECK: declare dso_local void @objc_msgSend_stret(i8*, i8*, ...)
 // CHECK-NOT: declare dllimport void @objc_msgSend(i8*, i8*, ...)
-
Index: CodeGenObjC/gnu-init.m
===
--- CodeGenObjC/gnu-init.m
+++ CodeGenObjC/gnu-init.m
@@ -99,6 +99,6 @@
 // Check our load function is in a comdat.
 // CHECK-WIN: define linkonce_odr hidden void @.objcv2_load_function() comdat {
 
-// Make sure we have dllimport on the load function
-// CHECK-WIN: declare dllimport void @__objc_load
+// Make sure we do not have dllimport on the load function
+// CHECK-WIN: declare dso_local void @__objc_load
 
Index: CodeGenCXX/runtime-dllstorage.cpp
===
--- CodeGenCXX/runtime-dllstorage.cpp
+++ CodeGenCXX/runtime-dllstorage.cpp
@@ -108,7 +108,7 @@
 // CHECK-MS-DAG: @_Init_thread_epoch = external thread_local global i32
 // CHECK-MS-DAG: declare dso_local i32 @__tlregdtor(void ()*)
 // CHECK-MS-DAG: declare dso_local i32 @atexit(void ()*)
-// CHECK-MS-DYNAMIC-DAG: declare dllimport {{.*}} void @_CxxThrowException
+// CHECK-MS-DYNAMIC-DAG: declare {{.*}} void 

[PATCH] D55269: [CUDA][OpenMP] Fix nvidia-cuda-toolkit detection on Debian/Ubuntu

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

jdenny added a comment.

By the way, nvidia-cuda-toolkit does install the following, but there's no nvvm 
directory as clang currently expects:

  /usr/lib/nvidia-cuda-toolkit
  /usr/lib/nvidia-cuda-toolkit/bin
  /usr/lib/nvidia-cuda-toolkit/bin/cicc
  /usr/lib/nvidia-cuda-toolkit/bin/crt
  /usr/lib/nvidia-cuda-toolkit/bin/crt/link.stub
  /usr/lib/nvidia-cuda-toolkit/bin/crt/prelink.stub
  /usr/lib/nvidia-cuda-toolkit/bin/g++
  /usr/lib/nvidia-cuda-toolkit/bin/gcc
  /usr/lib/nvidia-cuda-toolkit/bin/nvcc
  /usr/lib/nvidia-cuda-toolkit/bin/nvcc.profile
  /usr/lib/nvidia-cuda-toolkit/libdevice
  /usr/lib/nvidia-cuda-toolkit/libdevice/libdevice.10.bc


Repository:
  rC Clang

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

https://reviews.llvm.org/D55269



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

[PATCH] D55269: [CUDA][OpenMP] Fix nvidia-cuda-toolkit detection on Debian/Ubuntu

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

jdenny added a comment.

@tra, @Hahnfeld: Thanks for your replies.

In D55269#1318901 , @tra wrote:

> I'm not sure that's something that needs to be fixed in clang.
>
> IIUIC, Debian has added a shim that pretends to be a monolithic CUDA install:
>  https://bugs.launchpad.net/ubuntu/+source/clang/+bug/1706326
>  That change seems to be in Ubuntu bionic (18.04) 
> https://packages.ubuntu.com/en/bionic/nvidia-cuda-toolkit


apt confirms that's what I have: nvidia-cuda-toolkit 9.1.85-3ubuntu1

> With that fix in place --cuda-path=/usr/lib/cuda should work.

Seems to.  To be clear, I'm trying to address the use case where cmake/clang 
finds the cuda installation automatically.

> --cuda-path=/usr was never supposed to work -- /usr is *not* the root of the 
> CUDA SDK.

/usr/lib/cuda/bin/nvcc doesn't exist, so that's probably why FindCUDA.cmake 
finds /usr/bin/nvcc (also installed by nvidia-cuda-toolkit).  Is it fair then 
to say that /usr/lib/cuda isn't the root either?

> I guess that just adding the check for isUbuntu() should make clang work on 
> Ubuntu 18.04+.

It fixes the first issue I reported.  It does not fix the second.

It seems that nvidia-cuda-toolkit still isn't installing a complete CUDA 
install in one location.  Even if it installed it all to /usr/lib/cuda, 
FindCUDA.cmake would probably still see /usr/bin/nvcc and assume /usr is the 
CUDA install root.

What's the path forward?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55269



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

r348299 - [PowerPC] Make no-PIC default to match GCC - CLANG

[Stefan Pintilie via cfe-commits]

Author: stefanp
Date: Tue Dec  4 12:15:37 2018
New Revision: 348299

URL: http://llvm.org/viewvc/llvm-project?rev=348299=rev
Log:
[PowerPC] Make no-PIC default to match GCC - CLANG

Make -fno-PIC default on PowerPC LE.

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/test/Driver/clang-offload-bundler.c
cfe/trunk/test/Driver/ppc-abi.c

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=348299=348298=348299=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Tue Dec  4 12:15:37 2018
@@ -2435,7 +2435,7 @@ bool Generic_GCC::isPICDefault() const {
   case llvm::Triple::x86_64:
 return getTriple().isOSWindows();
   case llvm::Triple::ppc64:
-  case llvm::Triple::ppc64le:
+// Big endian PPC is PIC by default
 return !getTriple().isOSBinFormatMachO() && !getTriple().isMacOSX();
   case llvm::Triple::mips64:
   case llvm::Triple::mips64el:

Modified: cfe/trunk/test/Driver/clang-offload-bundler.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang-offload-bundler.c?rev=348299=348298=348299=diff
==
--- cfe/trunk/test/Driver/clang-offload-bundler.c (original)
+++ cfe/trunk/test/Driver/clang-offload-bundler.c Tue Dec  4 12:15:37 2018
@@ -115,7 +115,7 @@
 // CK-TEXTI: // __CLANG_OFFLOAD_BUNDLEEND__ openmp-x86_64-pc-linux-gnu
 
 // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLESTART__ 
host-powerpc64le-ibm-linux-gnu
-// CK-TEXTLL: @A = global i32 0
+// CK-TEXTLL: @A = dso_local global i32 0
 // CK-TEXTLL: define {{.*}}@test_func()
 // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLEEND__ host-powerpc64le-ibm-linux-gnu
 // CK-TEXTLL: ; __CLANG_OFFLOAD_BUNDLESTART__ 
openmp-powerpc64le-ibm-linux-gnu

Modified: cfe/trunk/test/Driver/ppc-abi.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/ppc-abi.c?rev=348299=348298=348299=diff
==
--- cfe/trunk/test/Driver/ppc-abi.c (original)
+++ cfe/trunk/test/Driver/ppc-abi.c Tue Dec  4 12:15:37 2018
@@ -13,12 +13,12 @@
 // RUN: %clang -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
 // RUN:   -mcpu=a2q -mno-qpx | FileCheck -check-prefix=CHECK-ELFv1 %s
 // RUN: %clang -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
-// RUN:   -mabi=elfv2 | FileCheck -check-prefix=CHECK-ELFv2 %s
+// RUN:   -mabi=elfv2 | FileCheck -check-prefix=CHECK-ELFv2-BE %s
 
 // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ELFv2 %s
 // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
-// RUN:   -mabi=elfv1 | FileCheck -check-prefix=CHECK-ELFv1 %s
+// RUN:   -mabi=elfv1 | FileCheck -check-prefix=CHECK-ELFv1-LE %s
 // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
 // RUN:   -mabi=elfv2 | FileCheck -check-prefix=CHECK-ELFv2 %s
 // RUN: %clang -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 2>&1 \
@@ -26,8 +26,44 @@
 
 // CHECK-ELFv1: "-mrelocation-model" "pic" "-pic-level" "2"
 // CHECK-ELFv1: "-target-abi" "elfv1"
+// CHECK-ELFv1-LE: "-mrelocation-model" "static"
+// CHECK-ELFv1-LE: "-target-abi" "elfv1"
 // CHECK-ELFv1-QPX: "-mrelocation-model" "pic" "-pic-level" "2"
 // CHECK-ELFv1-QPX: "-target-abi" "elfv1-qpx"
-// CHECK-ELFv2: "-mrelocation-model" "pic" "-pic-level" "2"
+// CHECK-ELFv2: "-mrelocation-model" "static"
 // CHECK-ELFv2: "-target-abi" "elfv2"
+// CHECK-ELFv2-BE: "-mrelocation-model" "pic" "-pic-level" "2"
+// CHECK-ELFv2-BE: "-target-abi" "elfv2"
+
+// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ELFv1-PIC %s
+// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mabi=elfv1 | FileCheck -check-prefix=CHECK-ELFv1-PIC %s
+// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mabi=elfv1-qpx | FileCheck -check-prefix=CHECK-ELFv1-QPX-PIC %s
+// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mcpu=a2q | FileCheck -check-prefix=CHECK-ELFv1-QPX-PIC %s
+// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mcpu=a2 -mqpx | FileCheck -check-prefix=CHECK-ELFv1-QPX-PIC %s
+// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mcpu=a2q -mno-qpx | FileCheck -check-prefix=CHECK-ELFv1-PIC %s
+// RUN: %clang -fPIC -target powerpc64-unknown-linux-gnu %s -### -o %t.o 2>&1 \
+// RUN:   -mabi=elfv2 | FileCheck -check-prefix=CHECK-ELFv2-PIC %s
+
+// RUN: %clang -fPIC -target powerpc64le-unknown-linux-gnu %s -### -o %t.o 
2>&1 \
+// RUN:   

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> We should probably prefer %clang_analyze_cc1 to %clang_cc1 -analyze here too.

Ok, changed that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55289: [analyzer] MoveChecker Pt.5: Improve invalidation policies.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:545
+  if (ThisRegion != Region)
+if (std::find(Regions.begin(), Regions.end(), Region) != Regions.end())
+  State = removeFromState(State, Region);

This is clumsy. I think we shouldn't include non-invalidated regions in the 
`ExplicitRegions` array in the first place.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55289



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176687.
martong added a comment.

- Use clang_analyze_cc1


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp

Index: test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -0,0 +1,22 @@
+// We do not expect any error when one part of the triple is unknown, but other
+// known parts are equal.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// expected-no-diagnostics
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: test/Analysis/ctu-different-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-different-triples.cpp
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_analyze_cc1 -triple powerpc64-montavista-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// We expect an error in this file, but without a location.
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -32,6 +32,36 @@
 namespace cross_tu {
 
 namespace {
+
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const llvm::Triple , const llvm::Triple ) {
+  using llvm::Triple;
+  if (Lhs.getArch() != Triple::UnknownArch &&
+  Rhs.getArch() != Triple::UnknownArch && Lhs.getArch() != Rhs.getArch())
+return false;
+  if (Lhs.getSubArch() != Triple::NoSubArch &&
+  Rhs.getSubArch() != Triple::NoSubArch &&
+  Lhs.getSubArch() != Rhs.getSubArch())
+return false;
+  if (Lhs.getVendor() != Triple::UnknownVendor &&
+  Rhs.getVendor() != Triple::UnknownVendor &&
+  Lhs.getVendor() != Rhs.getVendor())
+return false;
+  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() &&
+  Lhs.getOS() != Rhs.getOS())
+return false;
+  if (Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Rhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Lhs.getEnvironment() != Rhs.getEnvironment())
+return false;
+  if (Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Rhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Lhs.getObjectFormat() != Rhs.getObjectFormat())
+return false;
+  return true;
+}
+
 // FIXME: This class is will be removed after the transition to llvm::Error.
 class IndexErrorCategory : public std::error_category {
 public:
@@ -55,6 +85,10 @@
   return "Failed to load external AST source.";
 case index_error_code::failed_to_generate_usr:
   return "Failed to generate USR.";
+case index_error_code::triple_mismatch:
+  return "Triple mismatch";
+case index_error_code::lang_mismatch:
+  return "Language mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -166,6 +200,31 @@
   assert(>getFileManager() ==
  >getASTContext().getSourceManager().getFileManager());
 
+  const llvm::Triple  = Context.getTargetInfo().getTriple();
+  const llvm::Triple  =
+  Unit->getASTContext().getTargetInfo().getTriple();
+  // The imported AST had been generated for a different target.
+  // Some parts of the triple in the loaded ASTContext can be unknown while the
+  // very same parts in the target ASTContext are known. Thus we check for the
+  // known parts only.
+  if 

[PATCH] D55289: [analyzer] MoveChecker Pt.5: Improve invalidation policies.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, george.karpenkov, szepet, 
rnkovacs, Szelethus.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, baloghadamsoftware.

If a moved-from object is passed into a conservatively evaluated function by 
pointer or by reference, we assume that the function may reset its state.

Make sure it doesn't apply to const pointers and const references. Add a test 
that demonstrates that it does apply to rvalue references.

Additionally, make sure that the object is invalidated when its contents change 
for reasons other than invalidation caused by evaluating a call conservatively. 
In particular, when the object's fields are manipulated directly, we should 
assume that some sort of reset may be happening.


Repository:
  rC Clang

https://reviews.llvm.org/D55289

Files:
  lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  test/Analysis/use-after-move.cpp

Index: test/Analysis/use-after-move.cpp
===
--- test/Analysis/use-after-move.cpp
+++ test/Analysis/use-after-move.cpp
@@ -116,6 +116,19 @@
   bool empty() const;
   bool isEmpty() const;
   operator bool() const;
+
+  void testUpdateField() {
+A a;
+A b = std::move(a);
+a.i = 1;
+a.foo(); // no-warning
+  }
+  void testUpdateFieldDouble() {
+A a;
+A b = std::move(a);
+a.d = 1.0;
+a.foo(); // no-warning
+  }
 };
 
 int bignum();
@@ -594,24 +607,50 @@
   foobar(a.getI(), std::move(a)); //no-warning
 }
 
-void not_known(A );
-void not_known(A *a);
+void not_known_pass_by_ref(A );
+void not_known_pass_by_const_ref(const A );
+void not_known_pass_by_rvalue_ref(A &);
+void not_known_pass_by_ptr(A *a);
+void not_known_pass_by_const_ptr(const A *a);
 
 void regionAndPointerEscapeTest() {
   {
 A a;
 A b;
 b = std::move(a);
-not_known(a);
-a.foo(); //no-warning
+not_known_pass_by_ref(a);
+a.foo(); // no-warning
+  }
+  {
+A a;
+A b;
+b = std::move(a); // expected-note{{Object 'a' is moved}}
+not_known_pass_by_const_ref(a);
+a.foo(); // expected-warning{{Method called on moved-from object 'a'}}
+ // expected-note@-1{{Method called on moved-from object 'a'}}
+  }
+  {
+A a;
+A b;
+b = std::move(a);
+not_known_pass_by_rvalue_ref(std::move(a));
+a.foo(); // no-warning
   }
   {
 A a;
 A b;
 b = std::move(a);
-not_known();
+not_known_pass_by_ptr();
 a.foo(); // no-warning
   }
+  {
+A a;
+A b;
+b = std::move(a); // expected-note{{Object 'a' is moved}}
+not_known_pass_by_const_ptr();
+a.foo(); // expected-warning{{Method called on moved-from object 'a'}}
+ // expected-note@-1{{Method called on moved-from object 'a'}}
+  }
 }
 
 // A declaration statement containing multiple declarations sequences the
Index: lib/StaticAnalyzer/Checkers/MoveChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -527,16 +527,28 @@
 ArrayRef ExplicitRegions,
 ArrayRef Regions, const LocationContext *LCtx,
 const CallEvent *Call) const {
-  // In case of an InstanceCall don't remove the ThisRegion from the GDM since
-  // it is handled in checkPreCall and checkPostCall.
-  const MemRegion *ThisRegion = nullptr;
-  if (const auto *IC = dyn_cast_or_null(Call)) {
-ThisRegion = IC->getCXXThisVal().getAsRegion();
-  }
-
-  for (const auto *Region : ExplicitRegions) {
-if (ThisRegion != Region)
-  State = removeFromState(State, Region);
+  if (Call) {
+// Relax invalidation upon function calls: only invalidate parameters
+// that are passed directly via non-const pointers or non-const references
+// or rvalue references.
+// In case of an InstanceCall don't invalidate the this-region since
+// it is fully handled in checkPreCall and checkPostCall.
+const MemRegion *ThisRegion = nullptr;
+if (const auto *IC = dyn_cast_or_null(Call))
+  ThisRegion = IC->getCXXThisVal().getAsRegion();
+
+// Explicit regions are the regions passed into the call directly, but
+// not all of them end up being invalidated. The ones that do appear in
+// the Regions array as well.
+for (const auto *Region : ExplicitRegions)
+  if (ThisRegion != Region)
+if (std::find(Regions.begin(), Regions.end(), Region) != Regions.end())
+  State = removeFromState(State, Region);
+  } else {
+// For invalidations that aren't caused by calls, assume nothing. In
+// particular, direct write into an object's field invalidates the status.
+for (const auto *Region : Regions)
+  State = removeFromState(State, Region->getBaseRegion());
   }
 
   return State;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176686.
martong marked an inline comment as done.
martong added a comment.

- Add a case to emitCrossTUDiagnostics


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp

Index: test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -0,0 +1,22 @@
+// We do not expect any error when one part of the triple is unknown, but other
+// known parts are equal.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// expected-no-diagnostics
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: test/Analysis/ctu-different-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-different-triples.cpp
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple powerpc64-montavista-linux-gnu -fsyntax-only \
+// RUN:   -analyze -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// We expect an error in this file, but without a location.
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -32,6 +32,36 @@
 namespace cross_tu {
 
 namespace {
+
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const llvm::Triple , const llvm::Triple ) {
+  using llvm::Triple;
+  if (Lhs.getArch() != Triple::UnknownArch &&
+  Rhs.getArch() != Triple::UnknownArch && Lhs.getArch() != Rhs.getArch())
+return false;
+  if (Lhs.getSubArch() != Triple::NoSubArch &&
+  Rhs.getSubArch() != Triple::NoSubArch &&
+  Lhs.getSubArch() != Rhs.getSubArch())
+return false;
+  if (Lhs.getVendor() != Triple::UnknownVendor &&
+  Rhs.getVendor() != Triple::UnknownVendor &&
+  Lhs.getVendor() != Rhs.getVendor())
+return false;
+  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() &&
+  Lhs.getOS() != Rhs.getOS())
+return false;
+  if (Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Rhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Lhs.getEnvironment() != Rhs.getEnvironment())
+return false;
+  if (Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Rhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Lhs.getObjectFormat() != Rhs.getObjectFormat())
+return false;
+  return true;
+}
+
 // FIXME: This class is will be removed after the transition to llvm::Error.
 class IndexErrorCategory : public std::error_category {
 public:
@@ -55,6 +85,10 @@
   return "Failed to load external AST source.";
 case index_error_code::failed_to_generate_usr:
   return "Failed to generate USR.";
+case index_error_code::triple_mismatch:
+  return "Triple mismatch";
+case index_error_code::lang_mismatch:
+  return "Language mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -166,6 +200,31 @@
   assert(>getFileManager() ==
  >getASTContext().getSourceManager().getFileManager());
 
+  const llvm::Triple  = Context.getTargetInfo().getTriple();
+  const llvm::Triple  =
+  Unit->getASTContext().getTargetInfo().getTriple();
+  // The imported AST had been generated for a different target.
+  // Some parts of the triple in the loaded ASTContext can be unknown while the
+  // very same parts in the target ASTContext 

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

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



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:212
+// diagnostics.
+Context.getDiagnostics().Report(diag::err_ctu_incompat_triple)
+<< Unit->getMainFileName() << TripleTo.str() << TripleFrom.str();

xazax.hun wrote:
> martong wrote:
> > xazax.hun wrote:
> > > I think we should not emit an error here. It should be up to the caller 
> > > (the user of the library) to decide if she wants to handle this as an 
> > > error, warnings, or just suppress these kinds of problems. I would rather 
> > > extend `emitCrossTUDiagnostics` as a shorthand for the user if emitting 
> > > an error is the desired behavior.
> >  I would prefer to exploit the capabilities of the `DiagEngine`, instead of 
> > extending the interface of `CrossTranslationUnitContext`.
> > By using a `DiagGroup` we can upgrade the warning to be an error, so I just 
> > changed it to be like that.
> You do not need the extend the interface. There is already a function for 
> that below. Also, the user might not want to display a warning either but do 
> something else, like generate a log message. 
Ah, okay, I missed that. Now I added a case to that function. Still, I think 
having a `DiagGroup` is useful anyway.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55067: [HIP] Fix offset of kernel argument for AMDGPU target

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D55067#1318810 , @arsenm wrote:

>


An OpenCL kernel may call another OpenCL kernel. I am wondering how do you pass 
arguments to the kernel callee.

A simpler solution would be not letting hipSetupArgument specify the offset. 
Since the kernel metadata contains the alignment info of kernel argument, HIP 
runtime is able to calculate the offset based on kernel metadata by itself. 
hipSetupArgument only needs to specify the index of the argument and its size.


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

https://reviews.llvm.org/D55067



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

[PATCH] D55067: [HIP] Fix offset of kernel argument for AMDGPU target

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D55067#1318959 , @rjmccall wrote:

> In D55067#1318810 , @arsenm wrote:
>
> > In D55067#1313419 , @rjmccall 
> > wrote:
> >
> > > I understand that it's copied into a properly-aligned local variable, but 
> > > if it affects how the function is called, that's also part of the ABI, 
> > > and it should be taken from the C alignment, not any vagaries of how 
> > > struct types are translated into IR.
> > >
> > > The other reason to stick with the C alignment value is that it's 
> > > actually stable across compiler reasons, whereas IRGen does not promise 
> > > to use the same IR type for a struct across compiler versions.
> > >
> > > Now, you're a GPU compiler, so maybe you don't have any ABI requirements, 
> > > or maybe they're weaker for kernel functions, in which case this is 
> > > basically irrelevant.  But if you do care about compatibility here, you 
> > > should be aiming to communicate the alignment of this parameter correctly.
> > >
> > > This is part of why we largely try to avoid using arbitrary first-class 
> > > aggregate as parameters in IR.
> >
> >
> > byval isn't suitable for the purposes of the entry point here, so I don't 
> > see what the alternative is with the current constraints. We've defined the 
> > ABI input buffer as just the ABI allocation size/alignment of the IR type 
> > in order. Clang needs to get this to match what it wants.
>
>
> What I have said, repeatedly, is that that is not a good ABI rule because the 
> alignment of the IR type is not guaranteed to be meaningful or even stable.  
> Maybe there are good reasons why you don't care about having a stable ABI, 
> but I need you to at least acknowledge that that's what you're doing, because 
> otherwise it is hard for me to approve a patch that I know has this problem.


I don't understand how the alignment of the IR type isn't meaningful or stable. 
The DataLayout gives us the concept of an ABI alignment, but you seem to be 
saying this is meaningless (in which case why do we have it?)

I think we have different understandings of what an ABI means in this case. The 
"call" to a kernel isn't made in a vacuum unlike a normal function. For OpenCL 
at least, the backend produces metadata in the object file telling the explicit 
offsets of the individual arguments, which we today compute from the IR 
argument list. The actual caller isn't using the types to decide where to place 
arguments, the binary is explicitly telling it where. For some reason CUDA 
seems to be doing something different here which might be part of my confusion?

> 
> 
>> The alternative I would like is to stop using the IR argument list at all 
>> for kernels. All arguments would be accessed from offsets from an intrinsic 
>> call
> 
> That seems like a very reasonable alternative, or you could have the function 
> take a single pointer argument and do all accesses relative to that.  I'll 
> leave that up to you.

We would need to make up some other way to track the individual arguments, 
which I haven't come up with a great solution for (other than keeping them 
there without uses, which seems pretty awkward). As an optimization we 
currently do lower the kernel argument uses to look like this, but we compute 
the offsets from the IR arguments.


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

https://reviews.llvm.org/D55067



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

[PATCH] D55257: Inline handling of DependentSizedArrayType

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D55257#1318769 , @aaron.ballman 
wrote:

> In D55257#1318376 , @steveire wrote:
>
> > In D55257#1318328 , @aaron.ballman 
> > wrote:
> >
> > > > It is necessary to perform all printing before any traversal to child 
> > > > nodes.
> > >
> > > This piqued my interest -- is `VisitFunctionDecl()` then incorrect 
> > > because it streams output, then dumps parameter children, then dumps more 
> > > output, then dumps override children? Or do you mean "don't interleave 
> > > `VisitFoo()` calls with streaming output"?
> >
> >
> > Can you relate your question to https://reviews.llvm.org/D55083 ?
>
>
> Ah, I was looking at code before having fetched those changes, so perhaps my 
> example is poor. Mostly, I'm wondering what you meant by "traversal to child 
> nodes" -- do you mean:
>
> 1. it's bad to output to the stream, then dumpChild(), then output to the 
> stream again
> 2. it's bad to output to the stream, then VisitFoo(), then output to the 
> stream again
> 3. both #1 and #2
> 4. neither #1 nor #2
>
>   (as in: when I'm doing a code review a few months from now, what should I 
> be watching out for in this scenario?)


I don't think 'bad' is the right phrasing, but mostly your #1 is correct. 
VisitFoo() methods often call dumpChild() and even if one doesn't today, it can.

The reason to re-order is that it enables the separation of traversal from 
outputting. See the split for comments in https://reviews.llvm.org/D55190 and 
see that `dumpComment` there calls `NodeDumper.Visit`.

When the refactoring is finished, the traversal class will not have a output 
stream member, so it won't be able to stream to output, so there is nothing you 
need to keep in mind in 6 months.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55257



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

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 176677.
aprantl added a reviewer: ilya-biryukov.
aprantl added a comment.

Ilya, this updated revision should restore the original GCOV behavior both for 
absolute and relative paths.


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

https://reviews.llvm.org/D55085

Files:
  include/llvm/IR/DiagnosticInfo.h
  lib/IR/DiagnosticInfo.cpp
  lib/Transforms/Instrumentation/GCOVProfiling.cpp
  test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
  tools/clang/lib/CodeGen/CGDebugInfo.cpp
  tools/clang/lib/CodeGen/CodeGenAction.cpp
  tools/clang/test/CodeGen/debug-info-abspath.c
  tools/clang/test/CodeGen/debug-prefix-map.c
  tools/clang/test/Modules/module-debuginfo-prefix.m

Index: test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
===
--- test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
+++ test/CodeGen/AMDGPU/vi-removed-intrinsics.ll
@@ -17,7 +17,7 @@
 !llvm.module.flags = !{!2, !3}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_OpenCL, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug)
-!1 = !DIFile(filename: "foo.cl", directory: "/dev/null")
+!1 = !DIFile(filename: "foo.cl", directory: ".")
 !2 = !{i32 2, !"Dwarf Version", i32 4}
 !3 = !{i32 2, !"Debug Info Version", i32 3}
 !4 = !DILocation(line: 1, column: 42, scope: !5)
Index: lib/Transforms/Instrumentation/GCOVProfiling.cpp
===
--- lib/Transforms/Instrumentation/GCOVProfiling.cpp
+++ lib/Transforms/Instrumentation/GCOVProfiling.cpp
@@ -180,6 +180,21 @@
   return SP->getName();
 }
 
+/// Extract a filename for a DISubprogram.
+///
+/// Prefer relative paths in the coverage notes. Clang also may split
+/// up absolute paths into a directory and filename component. When
+/// the relative path doesn't exist, reconstruct the absolute path.
+SmallString<128> getFilename(const DISubprogram *SP) {
+  SmallString<128> Path;
+  StringRef RelPath = SP->getFilename();
+  if (sys::fs::exists(RelPath))
+Path = RelPath;
+  else
+sys::path::append(Path, SP->getDirectory(), SP->getFilename());
+  return Path;
+}
+
 namespace {
   class GCOVRecord {
protected:
@@ -256,7 +271,7 @@
 }
 
private:
-StringRef Filename;
+std::string Filename;
 SmallVector Lines;
   };
 
@@ -377,8 +392,9 @@
 
 void writeOut() {
   writeBytes(FunctionTag, 4);
+  SmallString<128> Filename = getFilename(SP);
   uint32_t BlockLen = 1 + 1 + 1 + lengthOfGCOVString(getFunctionName(SP)) +
-  1 + lengthOfGCOVString(SP->getFilename()) + 1;
+  1 + lengthOfGCOVString(Filename) + 1;
   if (UseCfgChecksum)
 ++BlockLen;
   write(BlockLen);
@@ -387,7 +403,7 @@
   if (UseCfgChecksum)
 write(CfgChecksum);
   writeGCOVString(getFunctionName(SP));
-  writeGCOVString(SP->getFilename());
+  writeGCOVString(Filename);
   write(SP->getLine());
 
   // Emit count of blocks.
@@ -466,7 +482,7 @@
   if (FilterRe.empty() && ExcludeRe.empty()) {
 return true;
   }
-  const StringRef Filename = F.getSubprogram()->getFilename();
+  SmallString<128> Filename = getFilename(F.getSubprogram());
   auto It = InstrumentedFiles.find(Filename);
   if (It != InstrumentedFiles.end()) {
 return It->second;
@@ -688,7 +704,8 @@
   // Add the function line number to the lines of the entry block
   // to have a counter for the function definition.
   uint32_t Line = SP->getLine();
-  Func.getBlock().getFile(SP->getFilename()).addLine(Line);
+  auto Filename = getFilename(SP);
+  Func.getBlock().getFile(Filename).addLine(Line);
 
   for (auto  : F) {
 GCOVBlock  = Func.getBlock();
@@ -719,7 +736,7 @@
   if (SP != getDISubprogram(Loc.getScope()))
 continue;
 
-  GCOVLines  = Block.getFile(SP->getFilename());
+  GCOVLines  = Block.getFile(Filename);
   Lines.addLine(Loc.getLine());
 }
 Line = 0;
Index: lib/IR/DiagnosticInfo.cpp
===
--- lib/IR/DiagnosticInfo.cpp
+++ lib/IR/DiagnosticInfo.cpp
@@ -33,9 +33,10 @@
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
-#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 #include 
 #include 
@@ -106,7 +107,7 @@
 DiagnosticLocation::DiagnosticLocation(const DebugLoc ) {
   if (!DL)
 return;
-  Filename = DL->getFilename();
+  File = DL->getFile();
   Line = DL->getLine();
   Column = DL->getColumn();
 }
@@ -114,17 +115,36 @@
 DiagnosticLocation::DiagnosticLocation(const DISubprogram *SP) {
   if (!SP)
 return;
-  Filename = SP->getFilename();
+  
+  File = SP->getFile();
   Line = SP->getScopeLine();
  

[PATCH] D54592: [CStringChecker] evaluate explicit_bzero

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

P.S. I guess you can also include the regular `bzero()` here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54592



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

[PATCH] D54592: [CStringChecker] evaluate explicit_bzero

[Artem Dergachev via Phabricator via cfe-commits]

NoQ edited reviewers, added: NoQ; removed: dergachev.a.
NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2206-2207
+  // See if the size argument is zero.
+  const LocationContext *LCtx = C.getLocationContext();
+  SVal SizeVal = State->getSVal(Size, LCtx);
+  QualType SizeTy = Size->getType();

Modernize a bit: `SVal SizeVal = C.getSVal(Size)`.



Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2214-2220
+  // Get the value of the memory area.
+  SVal MemVal = State->getSVal(Mem, LCtx);
+  
+  // If the size is zero, there won't be any actual memory access,
+  // In this case we just return.
+  if (StateZeroSize && !StateNonZeroSize)
+return;

Let's flip these two. We don't need `MemVal` if we are about to return.



Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2215
+  // Get the value of the memory area.
+  SVal MemVal = State->getSVal(Mem, LCtx);
+  

Modernize that as well and drop the `LCtx` variable.



Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2231
+return;
+}
+

Why not finally finish this off with `memsetAux()` to actually zero out the 
memory?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54592



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

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

[Pierre van Houtryve via Phabricator via cfe-commits]

Pierre-vh updated this revision to Diff 176676.
Pierre-vh added a comment.

Here's the diff without the extra newline :)


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

https://reviews.llvm.org/D55226

Files:
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  test/Analysis/security-syntax-checks.m


Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -177,6 +177,11 @@
   strcpy(x, "abcd");
 }
 
+void test_strcpy_safe_2() {
+  struct {char s1[100];} s;
+  strcpy(s.s1, "hello");
+}
+
 //===--===
 // strcat()
 //===--===
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -651,13 +651,12 @@
 
   const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
  *Source = CE->getArg(1)->IgnoreImpCasts();
-  if (const auto *DeclRef = dyn_cast(Target))
-if (const auto *Array = dyn_cast(DeclRef->getType())) {
-  uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
-  if (const auto *String = dyn_cast(Source)) {
-if (ArraySize >= String->getLength() + 1)
-  return;
-  }
+
+  if (const auto *Array = dyn_cast(Target->getType())) {
+uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+if (const auto *String = dyn_cast(Source)) {
+  if (ArraySize >= String->getLength() + 1)
+return;
 }
 
   // Issue a warning.


Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -177,6 +177,11 @@
   strcpy(x, "abcd");
 }
 
+void test_strcpy_safe_2() {
+  struct {char s1[100];} s;
+  strcpy(s.s1, "hello");
+}
+
 //===--===
 // strcat()
 //===--===
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -651,13 +651,12 @@
 
   const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
  *Source = CE->getArg(1)->IgnoreImpCasts();
-  if (const auto *DeclRef = dyn_cast(Target))
-if (const auto *Array = dyn_cast(DeclRef->getType())) {
-  uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
-  if (const auto *String = dyn_cast(Source)) {
-if (ArraySize >= String->getLength() + 1)
-  return;
-  }
+
+  if (const auto *Array = dyn_cast(Target->getType())) {
+uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+if (const auto *String = dyn_cast(Source)) {
+  if (ArraySize >= String->getLength() + 1)
+return;
 }
 
   // Issue a warning.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55127: [OpenCL] Diagnose conflicting address spaces between template definition and its instantiation

[John McCall via Phabricator via cfe-commits]

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

LGTM with minor revisions.




Comment at: lib/Sema/SemaType.cpp:7206
+  // Do not deduce address spaces in templates.
+  T->isDependentType())
 return;

I think a better comment here would be something like "Don't deduce address 
spaces for dependent types because they might end up instantiating to a type 
with a different address space qualifier."



Comment at: lib/Sema/SemaType.cpp:7232
+  if (D.getContext() == DeclaratorContext::TemplateArgContext)
+// Do not deduce address space for non-pointee type in template arg.
+;

Anastasia wrote:
> rjmccall wrote:
> > I don't understand what you're trying to do here.  Template arguments may 
> > need to be directly qualified with an address-space sometimes, and if you 
> > prevent that unconditionally you're going to break all sorts of things, 
> > like partially-specializing a template based on the presence of an 
> > address-space qualifier.
> I want to prevent deduction of address spaces for template arguments (when 
> they are not specified explicitly).
> 
> Without this change, this example won't compile:
>   template 
>   void foo() {
> static __global T i;
>   }
>   foo(); // error because int here is deduced to __private int (so i 
> will have conflicting addr space quals)
> But I think it's perfectly reasonable to compile this example because the 
> addr space qual of `i` is specified to be `__global`.
> 
> Basically I am just trying to fix OpenCL C deduction rules that didn't 
> account for the logic of templates.
> 
Okay.  Please use braces instead of a `;`.


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

https://reviews.llvm.org/D55127



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

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:1304
+RHSTy = ResultTy;
+  }
+

leonardchan wrote:
> rjmccall wrote:
> > leonardchan wrote:
> > > rjmccall wrote:
> > > > ebevhan wrote:
> > > > > rjmccall wrote:
> > > > > > leonardchan wrote:
> > > > > > > rjmccall wrote:
> > > > > > > > ebevhan wrote:
> > > > > > > > > rjmccall wrote:
> > > > > > > > > > Hmm.  So adding a signed integer to an unsigned fixed-point 
> > > > > > > > > > type always converts the integer to unsigned?  That's a 
> > > > > > > > > > little weird, but only because the fixed-point rule seems 
> > > > > > > > > > to be the other way.  Anyway, I assume it's not a bug in 
> > > > > > > > > > the spec.
> > > > > > > > > `handleFixedPointConversion` only calculates the result type 
> > > > > > > > > of the expression, not the calculation type. The final result 
> > > > > > > > > type of the operation will be the unsigned fixed-point type, 
> > > > > > > > > but the calculation itself will be done in a signed type with 
> > > > > > > > > enough precision to represent both the signed integer and the 
> > > > > > > > > unsigned fixed-point type. 
> > > > > > > > > 
> > > > > > > > > Though, if the result ends up being negative, the final 
> > > > > > > > > result is undefined unless the type is saturating. I don't 
> > > > > > > > > think there is a test for the saturating case (signed integer 
> > > > > > > > > + unsigned saturating fixed-point) in the SaturatedAddition 
> > > > > > > > > tests.
> > > > > > > > > `handleFixedPointConversion` only calculates the result type 
> > > > > > > > > of the expression, not the calculation type.
> > > > > > > > 
> > > > > > > > Right, I understand that, but the result type of the expression 
> > > > > > > > obviously impacts the expressive range of the result, since you 
> > > > > > > > can end up with a negative value.
> > > > > > > > 
> > > > > > > > Hmm.  With that said, if the general principle is to perform 
> > > > > > > > the operation with full precision on the original operand 
> > > > > > > > values, why are unsigned fixed-point operands coerced to their 
> > > > > > > > corresponding signed types *before* the operation?  This is 
> > > > > > > > precision-limiting if the unsigned representation doesn't use a 
> > > > > > > > padding bit.  That seems like a bug in the spec.
> > > > > > > > Hmm. With that said, if the general principle is to perform the 
> > > > > > > > operation with full precision on the original operand values, 
> > > > > > > > why are unsigned fixed-point operands coerced to their 
> > > > > > > > corresponding signed types *before* the operation? This is 
> > > > > > > > precision-limiting if the unsigned representation doesn't use a 
> > > > > > > > padding bit. That seems like a bug in the spec.
> > > > > > > 
> > > > > > > Possibly. When the standard mentions converting from signed to 
> > > > > > > unsigned fixed point, the only requirement involved is 
> > > > > > > conservation of magnitude (the number of integral bits excluding 
> > > > > > > the sign)
> > > > > > > 
> > > > > > > ```
> > > > > > > when signed and unsigned fixed-point types are mixed, the 
> > > > > > > unsigned type is converted to the corresponding signed type, and 
> > > > > > > this should go without loss of magnitude
> > > > > > > ```
> > > > > > > 
> > > > > > > This is just speculation, but I'm under the impression that not 
> > > > > > > as much "attention" was given for unsigned types as for signed 
> > > > > > > types since "`In the embedded processor world, support for 
> > > > > > > unsigned fixed-point data types is rare; normally only signed 
> > > > > > > fixed-point data types are supported`", but was kept anyway since 
> > > > > > > unsigned types are used a lot.
> > > > > > > 
> > > > > > > ```
> > > > > > > However, to disallow unsigned fixed-point arithmetic from 
> > > > > > > programming languages (in general, and from C in particular) 
> > > > > > > based on this observation, seems overly restrictive.
> > > > > > > ```
> > > > > > > 
> > > > > > > I also imagine that if the programmer needs more precision, the 
> > > > > > > correct approach would be to cast up to a type with a higher 
> > > > > > > scale. The standard seems to make an effort to expose as much in 
> > > > > > > regards to the underlying fixed point types as possible:
> > > > > > > 
> > > > > > > ```
> > > > > > > it should be possible to write fixed-point algorithms that are 
> > > > > > > independent of the actual fixed-point hardware support. This 
> > > > > > > implies that a programmer (or a running program) should have 
> > > > > > > access to all parameters that define the behavior of the 
> > > > > > > underlying hardware (in other words: even if these parameters are 
> > > > > > > implementation-defined).
> > > > > > > ```
> > > > > > > 
> > > > > > > So the user would at least know that unsigned types may not have 
> > > > > > > the 

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

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

Szelethus added a comment.

Thanks for the fix! :)

In D55226#1318853 , @Pierre-vh wrote:

> Hello again! I updated the diff and completely removed the outer if.  Please 
> let me know what you think!


If we're sure that `Target` isn't a `nullptr`, it's fine not to use a defensive 
check, but I think we definitely should add `assert(Target);` before using it.




Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:662
 
+
   // Issue a warning.

No need for this newline.


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

https://reviews.llvm.org/D55226



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

[PATCH] D55067: [HIP] Fix offset of kernel argument for AMDGPU target

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D55067#1318810 , @arsenm wrote:

> In D55067#1313419 , @rjmccall wrote:
>
> > I understand that it's copied into a properly-aligned local variable, but 
> > if it affects how the function is called, that's also part of the ABI, and 
> > it should be taken from the C alignment, not any vagaries of how struct 
> > types are translated into IR.
> >
> > The other reason to stick with the C alignment value is that it's actually 
> > stable across compiler reasons, whereas IRGen does not promise to use the 
> > same IR type for a struct across compiler versions.
> >
> > Now, you're a GPU compiler, so maybe you don't have any ABI requirements, 
> > or maybe they're weaker for kernel functions, in which case this is 
> > basically irrelevant.  But if you do care about compatibility here, you 
> > should be aiming to communicate the alignment of this parameter correctly.
> >
> > This is part of why we largely try to avoid using arbitrary first-class 
> > aggregate as parameters in IR.
>
>
> byval isn't suitable for the purposes of the entry point here, so I don't see 
> what the alternative is with the current constraints. We've defined the ABI 
> input buffer as just the ABI allocation size/alignment of the IR type in 
> order. Clang needs to get this to match what it wants.


What I have said, repeatedly, is that that is not a good ABI rule because the 
alignment of the IR type is not guaranteed to be meaningful or even stable.  
Maybe there are good reasons why you don't care about having a stable ABI, but 
I need you to at least acknowledge that that's what you're doing, because 
otherwise it is hard for me to approve a patch that I know has this problem.

> The alternative I would like is to stop using the IR argument list at all for 
> kernels. All arguments would be accessed from offsets from an intrinsic call

That seems like a very reasonable alternative, or you could have the function 
take a single pointer argument and do all accesses relative to that.  I'll 
leave that up to you.


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

https://reviews.llvm.org/D55067



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

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

Szelethus added a comment.

We should probably prefer `%clang_analyze_cc1` to `%clang_cc1 -analyze` here 
too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:212
+// diagnostics.
+Context.getDiagnostics().Report(diag::err_ctu_incompat_triple)
+<< Unit->getMainFileName() << TripleTo.str() << TripleFrom.str();

martong wrote:
> xazax.hun wrote:
> > I think we should not emit an error here. It should be up to the caller 
> > (the user of the library) to decide if she wants to handle this as an 
> > error, warnings, or just suppress these kinds of problems. I would rather 
> > extend `emitCrossTUDiagnostics` as a shorthand for the user if emitting an 
> > error is the desired behavior.
>  I would prefer to exploit the capabilities of the `DiagEngine`, instead of 
> extending the interface of `CrossTranslationUnitContext`.
> By using a `DiagGroup` we can upgrade the warning to be an error, so I just 
> changed it to be like that.
You do not need the extend the interface. There is already a function for that 
below. Also, the user might not want to display a warning either but do 
something else, like generate a log message. 


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping* @rsmith Any more comments on this patch or the one before it 
(https://reviews.llvm.org/D54014)? That one has the fix for pushing and popping 
another ExprEvalContext before acting on a function body in this patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49511



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176672.
martong added a comment.

- Forgot to rename err_ctu_incompat_triple -> warn_ctu_incompat_triple


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp

Index: test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -0,0 +1,22 @@
+// We do not expect any error when one part of the triple is unknown, but other
+// known parts are equal.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// expected-no-diagnostics
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: test/Analysis/ctu-different-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-different-triples.cpp
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple powerpc64-montavista-linux-gnu -fsyntax-only \
+// RUN:   -analyze -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// We expect an error in this file, but without a location.
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -32,6 +32,36 @@
 namespace cross_tu {
 
 namespace {
+
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const llvm::Triple , const llvm::Triple ) {
+  using llvm::Triple;
+  if (Lhs.getArch() != Triple::UnknownArch &&
+  Rhs.getArch() != Triple::UnknownArch && Lhs.getArch() != Rhs.getArch())
+return false;
+  if (Lhs.getSubArch() != Triple::NoSubArch &&
+  Rhs.getSubArch() != Triple::NoSubArch &&
+  Lhs.getSubArch() != Rhs.getSubArch())
+return false;
+  if (Lhs.getVendor() != Triple::UnknownVendor &&
+  Rhs.getVendor() != Triple::UnknownVendor &&
+  Lhs.getVendor() != Rhs.getVendor())
+return false;
+  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() &&
+  Lhs.getOS() != Rhs.getOS())
+return false;
+  if (Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Rhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Lhs.getEnvironment() != Rhs.getEnvironment())
+return false;
+  if (Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Rhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Lhs.getObjectFormat() != Rhs.getObjectFormat())
+return false;
+  return true;
+}
+
 // FIXME: This class is will be removed after the transition to llvm::Error.
 class IndexErrorCategory : public std::error_category {
 public:
@@ -55,6 +85,10 @@
   return "Failed to load external AST source.";
 case index_error_code::failed_to_generate_usr:
   return "Failed to generate USR.";
+case index_error_code::triple_mismatch:
+  return "Triple mismatch";
+case index_error_code::lang_mismatch:
+  return "Language mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -166,6 +200,30 @@
   assert(>getFileManager() ==
  >getASTContext().getSourceManager().getFileManager());
 
+  const auto  = Context.getTargetInfo().getTriple();
+  const auto  = Unit->getASTContext().getTargetInfo().getTriple();
+  // The imported AST had been generated for a different target.
+  // Some parts of the triple in the loaded ASTContext can be unknown while the
+  // very same parts in the target ASTContext are known. Thus we check for the
+  

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: include/clang/Basic/DiagnosticCrossTUKinds.td:19
+
+def err_ctu_incompat_triple : Error<
+  "imported AST from '%0' had been generated for a different target, current: 
%1, imported: %2">;

xazax.hun wrote:
> I am not sure if we want this to be an error. The analysis could continue 
> without any problems if we do not actually merge the two ASTs. So maybe 
> having a warning only is better. My concern is that once we have this error, 
> there would be no way to analyze mixed language (C/C++) projects cleanly 
> using CTU.
Okay, I agree, I changed it to be a warning by default.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:212
+// diagnostics.
+Context.getDiagnostics().Report(diag::err_ctu_incompat_triple)
+<< Unit->getMainFileName() << TripleTo.str() << TripleFrom.str();

xazax.hun wrote:
> I think we should not emit an error here. It should be up to the caller (the 
> user of the library) to decide if she wants to handle this as an error, 
> warnings, or just suppress these kinds of problems. I would rather extend 
> `emitCrossTUDiagnostics` as a shorthand for the user if emitting an error is 
> the desired behavior.
 I would prefer to exploit the capabilities of the `DiagEngine`, instead of 
extending the interface of `CrossTranslationUnitContext`.
By using a `DiagGroup` we can upgrade the warning to be an error, so I just 
changed it to be like that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176671.
martong marked 4 inline comments as done.
martong added a comment.

- Diagnose a warning (which may be upgradable to an error)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp

Index: test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -0,0 +1,22 @@
+// We do not expect any error when one part of the triple is unknown, but other
+// known parts are equal.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// expected-no-diagnostics
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: test/Analysis/ctu-different-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-different-triples.cpp
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple powerpc64-montavista-linux-gnu -fsyntax-only \
+// RUN:   -analyze -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// We expect an error in this file, but without a location.
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -32,6 +32,36 @@
 namespace cross_tu {
 
 namespace {
+
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const llvm::Triple , const llvm::Triple ) {
+  using llvm::Triple;
+  if (Lhs.getArch() != Triple::UnknownArch &&
+  Rhs.getArch() != Triple::UnknownArch && Lhs.getArch() != Rhs.getArch())
+return false;
+  if (Lhs.getSubArch() != Triple::NoSubArch &&
+  Rhs.getSubArch() != Triple::NoSubArch &&
+  Lhs.getSubArch() != Rhs.getSubArch())
+return false;
+  if (Lhs.getVendor() != Triple::UnknownVendor &&
+  Rhs.getVendor() != Triple::UnknownVendor &&
+  Lhs.getVendor() != Rhs.getVendor())
+return false;
+  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() &&
+  Lhs.getOS() != Rhs.getOS())
+return false;
+  if (Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Rhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Lhs.getEnvironment() != Rhs.getEnvironment())
+return false;
+  if (Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Rhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Lhs.getObjectFormat() != Rhs.getObjectFormat())
+return false;
+  return true;
+}
+
 // FIXME: This class is will be removed after the transition to llvm::Error.
 class IndexErrorCategory : public std::error_category {
 public:
@@ -55,6 +85,10 @@
   return "Failed to load external AST source.";
 case index_error_code::failed_to_generate_usr:
   return "Failed to generate USR.";
+case index_error_code::triple_mismatch:
+  return "Triple mismatch";
+case index_error_code::lang_mismatch:
+  return "Language mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -166,6 +200,30 @@
   assert(>getFileManager() ==
  >getASTContext().getSourceManager().getFileManager());
 
+  const auto  = Context.getTargetInfo().getTriple();
+  const auto  = Unit->getASTContext().getTargetInfo().getTriple();
+  // The imported AST had been generated for a different target.
+  // Some parts of the triple in the loaded ASTContext can be unknown while the
+  // very same parts in the target ASTContext are 

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

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

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

Thanks! LGTM! :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55135



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

[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/abseil-duration-subtraction.rst:7
+Checks for cases where subtraction should be performed in the
+``absl::Duration`` domain.  When subtracting two values, and the first one is
+known to be a conversion from ``absl::Duration``, we can infer that the second

Please fix double space.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55245



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

[PATCH] D55269: [CUDA][OpenMP] Fix nvidia-cuda-toolkit detection on Debian/Ubuntu

[Artem Belevich via Phabricator via cfe-commits]

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

I'm not sure that's something that needs to be fixed in clang.

IIUIC, Debian has added a shim that pretends to be a monolithic CUDA install:
https://bugs.launchpad.net/ubuntu/+source/clang/+bug/1706326
That change seems to be in Ubuntu bionic (18.04) 
https://packages.ubuntu.com/en/bionic/nvidia-cuda-toolkit
With that fix in place --cuda-path=/usr/lib/cuda should work.

--cuda-path=/usr was never supposed to work -- /usr is *not* the root of the 
CUDA SDK.

I guess that just adding the check for isUbuntu() should make clang work on 
Ubuntu 18.04+.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55269



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

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

[Pierre van Houtryve via Phabricator via cfe-commits]

Pierre-vh updated this revision to Diff 176661.
Pierre-vh added a comment.

Hello again! I updated the diff and completely removed the outer if.  Please 
let me know what you think!


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

https://reviews.llvm.org/D55226

Files:
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  test/Analysis/security-syntax-checks.m


Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -177,6 +177,11 @@
   strcpy(x, "abcd");
 }
 
+void test_strcpy_safe_2() {
+  struct {char s1[100];} s;
+  strcpy(s.s1, "hello");
+}
+
 //===--===
 // strcat()
 //===--===
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -651,15 +651,15 @@
 
   const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
  *Source = CE->getArg(1)->IgnoreImpCasts();
-  if (const auto *DeclRef = dyn_cast(Target))
-if (const auto *Array = dyn_cast(DeclRef->getType())) {
-  uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
-  if (const auto *String = dyn_cast(Source)) {
-if (ArraySize >= String->getLength() + 1)
-  return;
-  }
+
+  if (const auto *Array = dyn_cast(Target->getType())) {
+uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+if (const auto *String = dyn_cast(Source)) {
+  if (ArraySize >= String->getLength() + 1)
+return;
 }
 
+
   // Issue a warning.
   PathDiagnosticLocation CELoc =
 PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);


Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -177,6 +177,11 @@
   strcpy(x, "abcd");
 }
 
+void test_strcpy_safe_2() {
+  struct {char s1[100];} s;
+  strcpy(s.s1, "hello");
+}
+
 //===--===
 // strcat()
 //===--===
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -651,15 +651,15 @@
 
   const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
  *Source = CE->getArg(1)->IgnoreImpCasts();
-  if (const auto *DeclRef = dyn_cast(Target))
-if (const auto *Array = dyn_cast(DeclRef->getType())) {
-  uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
-  if (const auto *String = dyn_cast(Source)) {
-if (ArraySize >= String->getLength() + 1)
-  return;
-  }
+
+  if (const auto *Array = dyn_cast(Target->getType())) {
+uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+if (const auto *String = dyn_cast(Source)) {
+  if (ArraySize >= String->getLength() + 1)
+return;
 }
 
+
   // Issue a warning.
   PathDiagnosticLocation CELoc =
 PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53655: [ASTImporter] Fix redecl chain of classes and class templates

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@rsmith ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D53655



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

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[Leonard Chan via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaExpr.cpp:1304
+RHSTy = ResultTy;
+  }
+

rjmccall wrote:
> leonardchan wrote:
> > rjmccall wrote:
> > > ebevhan wrote:
> > > > rjmccall wrote:
> > > > > leonardchan wrote:
> > > > > > rjmccall wrote:
> > > > > > > ebevhan wrote:
> > > > > > > > rjmccall wrote:
> > > > > > > > > Hmm.  So adding a signed integer to an unsigned fixed-point 
> > > > > > > > > type always converts the integer to unsigned?  That's a 
> > > > > > > > > little weird, but only because the fixed-point rule seems to 
> > > > > > > > > be the other way.  Anyway, I assume it's not a bug in the 
> > > > > > > > > spec.
> > > > > > > > `handleFixedPointConversion` only calculates the result type of 
> > > > > > > > the expression, not the calculation type. The final result type 
> > > > > > > > of the operation will be the unsigned fixed-point type, but the 
> > > > > > > > calculation itself will be done in a signed type with enough 
> > > > > > > > precision to represent both the signed integer and the unsigned 
> > > > > > > > fixed-point type. 
> > > > > > > > 
> > > > > > > > Though, if the result ends up being negative, the final result 
> > > > > > > > is undefined unless the type is saturating. I don't think there 
> > > > > > > > is a test for the saturating case (signed integer + unsigned 
> > > > > > > > saturating fixed-point) in the SaturatedAddition tests.
> > > > > > > > `handleFixedPointConversion` only calculates the result type of 
> > > > > > > > the expression, not the calculation type.
> > > > > > > 
> > > > > > > Right, I understand that, but the result type of the expression 
> > > > > > > obviously impacts the expressive range of the result, since you 
> > > > > > > can end up with a negative value.
> > > > > > > 
> > > > > > > Hmm.  With that said, if the general principle is to perform the 
> > > > > > > operation with full precision on the original operand values, why 
> > > > > > > are unsigned fixed-point operands coerced to their corresponding 
> > > > > > > signed types *before* the operation?  This is precision-limiting 
> > > > > > > if the unsigned representation doesn't use a padding bit.  That 
> > > > > > > seems like a bug in the spec.
> > > > > > > Hmm. With that said, if the general principle is to perform the 
> > > > > > > operation with full precision on the original operand values, why 
> > > > > > > are unsigned fixed-point operands coerced to their corresponding 
> > > > > > > signed types *before* the operation? This is precision-limiting 
> > > > > > > if the unsigned representation doesn't use a padding bit. That 
> > > > > > > seems like a bug in the spec.
> > > > > > 
> > > > > > Possibly. When the standard mentions converting from signed to 
> > > > > > unsigned fixed point, the only requirement involved is conservation 
> > > > > > of magnitude (the number of integral bits excluding the sign)
> > > > > > 
> > > > > > ```
> > > > > > when signed and unsigned fixed-point types are mixed, the unsigned 
> > > > > > type is converted to the corresponding signed type, and this should 
> > > > > > go without loss of magnitude
> > > > > > ```
> > > > > > 
> > > > > > This is just speculation, but I'm under the impression that not as 
> > > > > > much "attention" was given for unsigned types as for signed types 
> > > > > > since "`In the embedded processor world, support for unsigned 
> > > > > > fixed-point data types is rare; normally only signed fixed-point 
> > > > > > data types are supported`", but was kept anyway since unsigned 
> > > > > > types are used a lot.
> > > > > > 
> > > > > > ```
> > > > > > However, to disallow unsigned fixed-point arithmetic from 
> > > > > > programming languages (in general, and from C in particular) based 
> > > > > > on this observation, seems overly restrictive.
> > > > > > ```
> > > > > > 
> > > > > > I also imagine that if the programmer needs more precision, the 
> > > > > > correct approach would be to cast up to a type with a higher scale. 
> > > > > > The standard seems to make an effort to expose as much in regards 
> > > > > > to the underlying fixed point types as possible:
> > > > > > 
> > > > > > ```
> > > > > > it should be possible to write fixed-point algorithms that are 
> > > > > > independent of the actual fixed-point hardware support. This 
> > > > > > implies that a programmer (or a running program) should have access 
> > > > > > to all parameters that define the behavior of the underlying 
> > > > > > hardware (in other words: even if these parameters are 
> > > > > > implementation-defined).
> > > > > > ```
> > > > > > 
> > > > > > So the user would at least know that unsigned types may not have 
> > > > > > the same scale as their corresponding signed types if the hardware 
> > > > > > handles them with different scales.
> > > > > > 
> > > > > > Also added test for signed integer 

[PATCH] D55129: [CTU] Eliminate race condition in CTU lit tests

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176660.
martong added a comment.

- Use rm, mkdir and break long RUN lines


Repository:
  rC Clang

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

https://reviews.llvm.org/D55129

Files:
  test/Analysis/ctu-main.cpp


Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -1,8 +1,15 @@
-// RUN: mkdir -p %T/ctudir
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o 
%T/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o 
%T/ctudir/ctu-chain.cpp.ast %S/Inputs/ctu-chain.cpp
-// RUN: cp %S/Inputs/externalFnMap.txt %T/ctudir/
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze 
-analyzer-checker=core,debug.ExprInspection -analyzer-config 
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir 
-verify %s
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-chain.cpp.ast %S/Inputs/ctu-chain.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/
+// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -verify %s
 
 #include "ctu-hdr.h"
 


Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -1,8 +1,15 @@
-// RUN: mkdir -p %T/ctudir
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %T/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %T/ctudir/ctu-chain.cpp.ast %S/Inputs/ctu-chain.cpp
-// RUN: cp %S/Inputs/externalFnMap.txt %T/ctudir/
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir -verify %s
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-chain.cpp.ast %S/Inputs/ctu-chain.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/
+// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -verify %s
 
 #include "ctu-hdr.h"
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits