[PATCH] D61816: [CFG] [analyzer] pr41300: Add a branch to skip virtual base initializers when they are handled by the superclass.

[Csaba Dabis via Phabricator via cfe-commits]

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

I do not like to have a variable storing/mention one stuff named in plural. It 
is your decision as it is  just my personal feeling.




Comment at: clang/include/clang/Analysis/AnalysisDeclContext.h:462
  bool markElidedCXXConstructors = true,
+ bool addVirtualBasesBranches = true,
  CodeInjector *injector = nullptr);

`Bases` -> `Base`



Comment at: clang/include/clang/Analysis/CFG.h:510
+/// constructor.
+VirtualBasesBranch,
 

The most derived class instead. `Bases` -> `Base`



Comment at: clang/include/clang/Analysis/CFG.h:541
+return getKind() == VirtualBasesBranch;
+  }
 };

`Bases` -> `Base`



Comment at: clang/include/clang/Analysis/CFG.h:1050
 bool MarkElidedCXXConstructors = false;
+bool AddVirtualBasesBranches = false;
 

`Bases` -> `Base`



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h:120
+  void HandleVirtualBasesBranch(const CFGBlock *B, ExplodedNode *Pred);
+
 private:

`Bases` -> `Base`



Comment at: clang/lib/Analysis/AnalysisDeclContext.cpp:73
 bool addRichCXXConstructors, bool markElidedCXXConstructors,
-CodeInjector *injector)
+bool addVirtualBasesBranches, CodeInjector *injector)
 : Injector(injector), FunctionBodyFarm(ASTCtx, injector),

`Bases` -> `Base`



Comment at: clang/lib/Analysis/CFG.cpp:1435
+  // For C++ constructor add initializers to CFG. Virtual base classes may have
+  // already been initialized by the superclass. Make a branch so that it was
+  // possible to jump straight to non-virtual bases and fields, skipping

The most derived class instead.



Comment at: clang/lib/Analysis/CFG.cpp:1794
+// TODO: Add a VirtualBasesBranch to see if the superclass is responsible
+// for destroying them.
 const CXXRecordDecl *CD = VI.getType()->getAsCXXRecordDecl();

`Bases` -> `Base`



Comment at: clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp:40
+  /*addVirtualBasesBranches=*/true,
+  injector),
   Ctx(ASTCtx), Diags(diags), LangOpts(ASTCtx.getLangOpts()),

`Bases` -> `Base`



Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:447
 
+void CoreEngine::HandleVirtualBasesBranch(const CFGBlock *B,
+  ExplodedNode *Pred) {

`Bases` -> `Base`



Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:451
+  if (const auto *CallerCtor = dyn_cast_or_null(
+  LCtx->getStackFrame()->getCallSite())) {
+switch (CallerCtor->getConstructionKind()) {

May it is worth to handle the call outside to see whether it is non-null.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:435
+const auto *OuterCtor = dyn_cast_or_null(
+LCtx->getStackFrame()->getCallSite());
+assert(

May it is worth to handle the call outside to see whether it is non-null.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:440
+ OuterCtor->getConstructionKind() == CXXConstructExpr::CK_Delegating) 
&&
+("This virtual base should have been initialized by the superclass!"));
+(void)OuterCtor;

The most derived class?


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

https://reviews.llvm.org/D61816



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

r360863 - [builtin] Fixed definitions of builtins that rely on the int/long long type is 32/64 bits

[Karl-Johan Karlsson via cfe-commits]

Author: karka
Date: Thu May 16 00:18:02 2019
New Revision: 360863

URL: http://llvm.org/viewvc/llvm-project?rev=360863&view=rev
Log:
[builtin] Fixed definitions of builtins that rely on the int/long long type is 
32/64 bits

Summary:
The definition of the builtins __builtin_bswap32, __builtin_bitreverse32, 
__builtin_rotateleft32 and __builtin_rotateright32 rely on that the int type is 
32 bits wide on the target.
The defintions of the builtins __builtin_bswap64, __builtin_bitreverse64, 
__builtin_rotateleft64, and __builtin_rotateright64 rely on that the long long 
type is 64 bits wide.

On targets where this is not the case (e.g. AVR) clang will generate faulty 
code (wrong llvm assembler intrinsics).

This patch add support for using 'Z' (the int32_t type) in Bultins.def. The 
builtins above are changed to be based on the int32_t type instead of the int 
type, and the int64_t type instead of the long long type.

The AVR backend (experimental) have a native int type that is only 16 bits 
wide. The supplied testcase will therefore fail if running the testcase on 
trunk as clang will convert e.g. __builtin_bitreverse32 into 
llvm.bitreverse.i16 on AVR.

Reviewers: dylanmckay, spatel, rsmith, efriedma

Reviewed By: efriedma

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

Added:
cfe/trunk/test/CodeGen/avr-builtins.c
cfe/trunk/test/CodeGen/builtins.cpp
Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/lib/AST/ASTContext.cpp

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=360863&r1=360862&r2=360863&view=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Thu May 16 00:18:02 2019
@@ -50,7 +50,8 @@
 //  L   -> long (e.g. Li for 'long int', Ld for 'long double')
 //  LL  -> long long (e.g. LLi for 'long long int', LLd for __float128)
 //  LLL -> __int128_t (e.g. LLLi)
-//  W   -> int64_t
+//  Z   -> int32_t (require a native 32-bit integer type on the target)
+//  W   -> int64_t (require a native 64-bit integer type on the target)
 //  N   -> 'int' size if target is LP64, 'L' otherwise.
 //  S   -> signed
 //  U   -> unsigned
@@ -418,25 +419,27 @@ BUILTIN(__builtin_clrsb  , "ii"  , "nc")
 BUILTIN(__builtin_clrsbl , "iLi" , "nc")
 BUILTIN(__builtin_clrsbll, "iLLi", "nc")
 
-// FIXME: These type signatures are not correct for targets with int != 32-bits
-// or with ULL != 64-bits.
+// The following builtins rely on that char == 8 bits, short == 16 bits and 
that
+// there exists native types on the target that are 32- and 64-bits wide, 
unless
+// these conditions are fulfilled these builtins will operate on a not intended
+// bitwidth.
 BUILTIN(__builtin_bswap16, "UsUs", "nc")
-BUILTIN(__builtin_bswap32, "UiUi", "nc")
-BUILTIN(__builtin_bswap64, "ULLiULLi", "nc")
+BUILTIN(__builtin_bswap32, "UZiUZi", "nc")
+BUILTIN(__builtin_bswap64, "UWiUWi", "nc")
 
 BUILTIN(__builtin_bitreverse8, "UcUc", "nc")
 BUILTIN(__builtin_bitreverse16, "UsUs", "nc")
-BUILTIN(__builtin_bitreverse32, "UiUi", "nc")
-BUILTIN(__builtin_bitreverse64, "ULLiULLi", "nc")
+BUILTIN(__builtin_bitreverse32, "UZiUZi", "nc")
+BUILTIN(__builtin_bitreverse64, "UWiUWi", "nc")
 
 BUILTIN(__builtin_rotateleft8, "UcUcUc", "nc")
 BUILTIN(__builtin_rotateleft16, "UsUsUs", "nc")
-BUILTIN(__builtin_rotateleft32, "UiUiUi", "nc")
-BUILTIN(__builtin_rotateleft64, "ULLiULLiULLi", "nc")
+BUILTIN(__builtin_rotateleft32, "UZiUZiUZi", "nc")
+BUILTIN(__builtin_rotateleft64, "UWiUWiUWi", "nc")
 BUILTIN(__builtin_rotateright8, "UcUcUc", "nc")
 BUILTIN(__builtin_rotateright16, "UsUsUs", "nc")
-BUILTIN(__builtin_rotateright32, "UiUiUi", "nc")
-BUILTIN(__builtin_rotateright64, "ULLiULLiULLi", "nc")
+BUILTIN(__builtin_rotateright32, "UZiUZiUZi", "nc")
+BUILTIN(__builtin_rotateright64, "UWiUWiWi", "nc")
 
 // Random GCC builtins
 BUILTIN(__builtin_constant_p, "i.", "nctu")

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=360863&r1=360862&r2=360863&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Thu May 16 00:18:02 2019
@@ -9286,7 +9286,7 @@ static QualType DecodeTypeFromStr(const
   // Read the prefixed modifiers first.
   bool Done = false;
   #ifndef NDEBUG
-  bool IsSpecialLong = false;
+  bool IsSpecial = false;
   #endif
   while (!Done) {
 switch (*Str++) {
@@ -9305,26 +9305,26 @@ static QualType DecodeTypeFromStr(const
   Unsigned = true;
   break;
 case 'L':
-  assert(!IsSpecialLong && "Can't use 'L' with 'W' or 'N' modifiers");
+  assert(!IsSpecial && "Can't use 'L' with 'W', 'N' or 'Z' modifiers");
   assert(HowLong <= 2 && "Can't have  modifier");
   ++HowLong;
   br

[PATCH] D61845: [builtin] Fixed definitions of builtins that rely on the int/long long type is 32/64 bits

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL360863: [builtin] Fixed definitions of builtins that rely on 
the int/long long type is… (authored by karka, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61845

Files:
  cfe/trunk/include/clang/Basic/Builtins.def
  cfe/trunk/lib/AST/ASTContext.cpp
  cfe/trunk/test/CodeGen/avr-builtins.c
  cfe/trunk/test/CodeGen/builtins.cpp

Index: cfe/trunk/lib/AST/ASTContext.cpp
===
--- cfe/trunk/lib/AST/ASTContext.cpp
+++ cfe/trunk/lib/AST/ASTContext.cpp
@@ -9286,7 +9286,7 @@
   // Read the prefixed modifiers first.
   bool Done = false;
   #ifndef NDEBUG
-  bool IsSpecialLong = false;
+  bool IsSpecial = false;
   #endif
   while (!Done) {
 switch (*Str++) {
@@ -9305,26 +9305,26 @@
   Unsigned = true;
   break;
 case 'L':
-  assert(!IsSpecialLong && "Can't use 'L' with 'W' or 'N' modifiers");
+  assert(!IsSpecial && "Can't use 'L' with 'W', 'N' or 'Z' modifiers");
   assert(HowLong <= 2 && "Can't have  modifier");
   ++HowLong;
   break;
 case 'N':
   // 'N' behaves like 'L' for all non LP64 targets and 'int' otherwise.
-  assert(!IsSpecialLong && "Can't use two 'N' or 'W' modifiers!");
+  assert(!IsSpecial && "Can't use two 'N', 'W' or 'Z' modifiers!");
   assert(HowLong == 0 && "Can't use both 'L' and 'N' modifiers!");
   #ifndef NDEBUG
-  IsSpecialLong = true;
+  IsSpecial = true;
   #endif
   if (Context.getTargetInfo().getLongWidth() == 32)
 ++HowLong;
   break;
 case 'W':
   // This modifier represents int64 type.
-  assert(!IsSpecialLong && "Can't use two 'N' or 'W' modifiers!");
+  assert(!IsSpecial && "Can't use two 'N', 'W' or 'Z'  modifiers!");
   assert(HowLong == 0 && "Can't use both 'L' and 'W' modifiers!");
   #ifndef NDEBUG
-  IsSpecialLong = true;
+  IsSpecial = true;
   #endif
   switch (Context.getTargetInfo().getInt64Type()) {
   default:
@@ -9337,6 +9337,27 @@
 break;
   }
   break;
+case 'Z':
+  // This modifier represents int32 type.
+  assert(!IsSpecial && "Can't use two 'N', 'W' or 'Z' modifiers!");
+  assert(HowLong == 0 && "Can't use both 'L' and 'Z' modifiers!");
+  #ifndef NDEBUG
+  IsSpecial = true;
+  #endif
+  switch (Context.getTargetInfo().getIntTypeByWidth(32, true)) {
+  default:
+llvm_unreachable("Unexpected integer type");
+  case TargetInfo::SignedInt:
+HowLong = 0;
+break;
+  case TargetInfo::SignedLong:
+HowLong = 1;
+break;
+  case TargetInfo::SignedLongLong:
+HowLong = 2;
+break;
+  }
+  break;
 }
   }
 
Index: cfe/trunk/include/clang/Basic/Builtins.def
===
--- cfe/trunk/include/clang/Basic/Builtins.def
+++ cfe/trunk/include/clang/Basic/Builtins.def
@@ -50,7 +50,8 @@
 //  L   -> long (e.g. Li for 'long int', Ld for 'long double')
 //  LL  -> long long (e.g. LLi for 'long long int', LLd for __float128)
 //  LLL -> __int128_t (e.g. LLLi)
-//  W   -> int64_t
+//  Z   -> int32_t (require a native 32-bit integer type on the target)
+//  W   -> int64_t (require a native 64-bit integer type on the target)
 //  N   -> 'int' size if target is LP64, 'L' otherwise.
 //  S   -> signed
 //  U   -> unsigned
@@ -418,25 +419,27 @@
 BUILTIN(__builtin_clrsbl , "iLi" , "nc")
 BUILTIN(__builtin_clrsbll, "iLLi", "nc")
 
-// FIXME: These type signatures are not correct for targets with int != 32-bits
-// or with ULL != 64-bits.
+// The following builtins rely on that char == 8 bits, short == 16 bits and that
+// there exists native types on the target that are 32- and 64-bits wide, unless
+// these conditions are fulfilled these builtins will operate on a not intended
+// bitwidth.
 BUILTIN(__builtin_bswap16, "UsUs", "nc")
-BUILTIN(__builtin_bswap32, "UiUi", "nc")
-BUILTIN(__builtin_bswap64, "ULLiULLi", "nc")
+BUILTIN(__builtin_bswap32, "UZiUZi", "nc")
+BUILTIN(__builtin_bswap64, "UWiUWi", "nc")
 
 BUILTIN(__builtin_bitreverse8, "UcUc", "nc")
 BUILTIN(__builtin_bitreverse16, "UsUs", "nc")
-BUILTIN(__builtin_bitreverse32, "UiUi", "nc")
-BUILTIN(__builtin_bitreverse64, "ULLiULLi", "nc")
+BUILTIN(__builtin_bitreverse32, "UZiUZi", "nc")
+BUILTIN(__builtin_bitreverse64, "UWiUWi", "nc")
 
 BUILTIN(__builtin_rotateleft8, "UcUcUc", "nc")
 BUILTIN(__builtin_rotateleft16, "UsUsUs", "nc")
-BUILTIN(__builtin_rotateleft32, "UiUiUi", "nc")
-BUILTIN(__builtin_rotateleft64, "ULLiULLiULLi", "nc")
+BUILTIN(__builtin_rotateleft32, "UZiUZiUZi", "nc")
+BUILTIN(__builtin_rotateleft64, "UWiUWiUWi", "nc")
 BUILTIN(__builtin_rotateright8, "UcUcUc", "nc")
 BUILTIN(__builtin_rotateright16, 

[PATCH] D61974: [ObjC] Fix encoding of ObjC pointer types that are pointers to typedefs

[David Chisnall via Phabricator via cfe-commits]

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



Comment at: lib/AST/ASTContext.cpp:6973
 QualType PointeeTy = OPT->getPointeeType();
-if (!Options.EncodingProperty() &&
+if (getLangOpts().ObjCRuntime.isGNUFamily() &&
+!Options.EncodingProperty() &&

Please can we at least make this check just for the GCC runtime?  I'm not sure 
it's even needed there.  I've previously had to write code that works around 
this and have always considered it a bug in GCC, rather than a feature that I'd 
like us to copy, so I'd also be happy with just deleting this code path 
entirely.



Repository:
  rC Clang

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

https://reviews.llvm.org/D61974



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

[clang-tools-extra] r360869 - [clang-tidy] Do not list enabled checks when -quiet is given to run-clang-tidy.

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Thu May 16 02:22:39 2019
New Revision: 360869

URL: http://llvm.org/viewvc/llvm-project?rev=360869&view=rev
Log:
[clang-tidy] Do not list enabled checks when -quiet is given to run-clang-tidy.

Summary: When run-clang-tidy is given the -quiet flag, do not output the 
potentially hundreds of enabled check names at the beginning.

Patch by svenpanne.

Reviewers: alexfh, JonasToth

Reviewed By: JonasToth

Subscribers: JonasToth, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py

Modified: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py?rev=360869&r1=360868&r2=360869&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py Thu May 16 
02:22:39 2019
@@ -245,7 +245,12 @@ def main():
 if args.checks:
   invocation.append('-checks=' + args.checks)
 invocation.append('-')
-subprocess.check_call(invocation)
+if args.quiet:
+  # Even with -quiet we still want to check if we can call clang-tidy.
+  with open(os.devnull, 'w') as dev_null:
+subprocess.check_call(invocation, stdout=dev_null)
+else:
+  subprocess.check_call(invocation)
   except:
 print("Unable to run clang-tidy.", file=sys.stderr)
 sys.exit(1)


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

[PATCH] D61849: Do not list enabled checks when -quiet is given to run-clang-tidy.

[Jonas Toth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL360869: [clang-tidy] Do not list enabled checks when -quiet 
is given to run-clang-tidy. (authored by JonasToth, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61849?vs=199229&id=199769#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61849

Files:
  clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
@@ -245,7 +245,12 @@
 if args.checks:
   invocation.append('-checks=' + args.checks)
 invocation.append('-')
-subprocess.check_call(invocation)
+if args.quiet:
+  # Even with -quiet we still want to check if we can call clang-tidy.
+  with open(os.devnull, 'w') as dev_null:
+subprocess.check_call(invocation, stdout=dev_null)
+else:
+  subprocess.check_call(invocation)
   except:
 print("Unable to run clang-tidy.", file=sys.stderr)
 sys.exit(1)


Index: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
@@ -245,7 +245,12 @@
 if args.checks:
   invocation.append('-checks=' + args.checks)
 invocation.append('-')
-subprocess.check_call(invocation)
+if args.quiet:
+  # Even with -quiet we still want to check if we can call clang-tidy.
+  with open(os.devnull, 'w') as dev_null:
+subprocess.check_call(invocation, stdout=dev_null)
+else:
+  subprocess.check_call(invocation)
   except:
 print("Unable to run clang-tidy.", file=sys.stderr)
 sys.exit(1)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61849: Do not list enabled checks when -quiet is given to run-clang-tidy.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Yes someone has to do it on behalf of you.
I did so for you. After a view good patches you can get commit rights from 
chris lattner, until then the reviewer can commit for you.

Thank you for the patch!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61849



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

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 199775.
ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59885

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Lex/Token.h
  clang/include/clang/Lex/TokenLexer.h
  clang/include/clang/Parse/Parser.h
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/lib/Lex/MacroArgs.cpp
  clang/lib/Lex/PPCaching.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Lex/TokenLexer.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Rewrite/HTMLRewrite.cpp

Index: clang/lib/Rewrite/HTMLRewrite.cpp
===
--- clang/lib/Rewrite/HTMLRewrite.cpp
+++ clang/lib/Rewrite/HTMLRewrite.cpp
@@ -572,7 +572,7 @@
 
   // Enter the tokens we just lexed.  This will cause them to be macro expanded
   // but won't enter sub-files (because we removed #'s).
-  TmpPP.EnterTokenStream(TokenStream, false);
+  TmpPP.EnterTokenStream(TokenStream, false, /*IsReinject=*/false);
 
   TokenConcatenation ConcatInfo(TmpPP);
 
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -1545,7 +1545,7 @@
   if (PP.isBacktrackEnabled())
 PP.RevertCachedTokens(1);
   else
-PP.EnterToken(Tok);
+PP.EnterToken(Tok, /*IsReinject=*/true);
   Tok.setKind(tok::annot_cxxscope);
   Tok.setAnnotationValue(Actions.SaveNestedNameSpecifierAnnotation(SS));
   Tok.setAnnotationRange(SS.getRange());
@@ -1764,7 +1764,7 @@
   Token TypedefToken;
   PP.Lex(TypedefToken);
   bool Result = TryAnnotateTypeOrScopeToken();
-  PP.EnterToken(Tok);
+  PP.EnterToken(Tok, /*IsReinject=*/true);
   Tok = TypedefToken;
   if (!Result)
 Diag(Tok.getLocation(), diag::warn_expected_qualified_after_typename);
Index: clang/lib/Parse/ParseTemplate.cpp
===
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -919,7 +919,7 @@
 PrevTokLocation = RAngleLoc;
   } else {
 PrevTokLocation = TokBeforeGreaterLoc;
-PP.EnterToken(Tok);
+PP.EnterToken(Tok, /*IsReinject=*/true);
 Tok = Greater;
   }
 
@@ -1402,7 +1402,7 @@
   // Append the current token at the end of the new token stream so that it
   // doesn't get lost.
   LPT.Toks.push_back(Tok);
-  PP.EnterTokenStream(LPT.Toks, true);
+  PP.EnterTokenStream(LPT.Toks, true, /*IsReinject*/true);
 
   // Consume the previously pushed token.
   ConsumeAnyToken(/*ConsumeCodeCompletionTok=*/true);
Index: clang/lib/Parse/ParseStmtAsm.cpp
===
--- clang/lib/Parse/ParseStmtAsm.cpp
+++ clang/lib/Parse/ParseStmtAsm.cpp
@@ -213,7 +213,8 @@
   // Also copy the current token over.
   LineToks.push_back(Tok);
 
-  PP.EnterTokenStream(LineToks, /*DisableMacroExpansions*/ true);
+  PP.EnterTokenStream(LineToks, /*DisableMacroExpansions*/ true,
+  /*IsReinject*/ true);
 
   // Clear the current token and advance to the first token in LineToks.
   ConsumeAnyToken();
Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -117,7 +117,8 @@
 Toks[0].setAnnotationEndLoc(Tok.getLocation());
 Toks[0].setAnnotationValue(reinterpret_cast(
static_cast(OOS)));
-PP.EnterTokenStream(Toks, /*DisableMacroExpansion=*/true);
+PP.EnterTokenStream(Toks, /*DisableMacroExpansion=*/true,
+/*IsReinject=*/false);
   }
 };
 
@@ -739,7 +740,8 @@
   // Grab the tokens out of the annotation and enter them into the stream.
   auto TheTokens =
   (std::pair, size_t> *)Tok.getAnnotationValue();
-  PP.EnterTokenStream(std::move(TheTokens->first), TheTokens->second, true);
+  PP.EnterTokenStream(std::move(TheTokens->first), TheTokens->second, true,
+  /*IsReinject=*/true);
   SourceLocation PragmaLocation = ConsumeAnnotationToken();
   assert(Tok.isAnyIdentifier());
   StringRef PragmaName = Tok.getIdentifierInfo()->getName();
@@ -,7 +1113,8 @@
 Hint.StateLoc = IdentifierLoc::create(Actions.Context, StateLoc, StateInfo);
   } else {
 // Enter constant expression i

[PATCH] D61722: [AST] Add RecoveryExpr; produce it on overload resolution failure and missing member.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 3 inline comments as done.
sammccall added a comment.

Thanks for the useful insights!

I'm going to work on a version of this patch that doesn't rely on type 
dependency and treats errors as a new concept instead.
I'm not convinced it's feasible to drop RecoveryExpr and reuse existing nodes, 
so I'll keep that at least for now. (I tried and failed, and don't have new 
ideas there - more detail below)

In D61722#1503863 , @rsmith wrote:

> I expect we'll want a `ContainsErrors` bit on `Stmt`, propagated similarly to 
> the existing `InstantiationDependent` / `ContainsUnexpandedParameterPack` / 
> ... bits.


This sounds good (as an alternative to using dependent bits for this).

Do you think we want a similar bit on `Type` (set on e.g. `vector`) , 
or should be ensure `ErrorTy` never gets used for compound types?

> For example, the constant evaluator will want to quickly bail out of 
> evaluating expressions and statements containing errors, and the error 
> recovery `TreeTransform` that we perform to fix typos will want that too (and 
> maybe could be able to fix other kinds of errors for which we build these 
> error nodes eventually?).

Definitely possible, I don't know whether it's worth it or not:

1. errors that can be recovered eagerly: They are, and `RecoveryExpr` or 
equivalent never kicks in here. This seems ideal.
2. errors that can never be recovered: returned as `RecoveryExpr` or 
equivalent. This is a difference from current TypoExpr which never does this.
3. errors that must be recovered lazily: we could e.g. add TypoExpr-style 
recovery callbacks to `RecoveryExpr` and merge the two concepts. I don't have a 
clear idea of how large/important this category is compared to 1 though.




Comment at: include/clang/AST/BuiltinTypes.def:265
+// a template.
+BUILTIN_TYPE(Recovery, RecoveryTy)
+

rsmith wrote:
> erik.pilkington wrote:
> > Why are you creating a new type as opposed to just using DependentTy (sorta 
> > like TypoExpr does)? It seems like if you want to recycle all the 
> > dependence-propagating code in Sema, then you need to fall back to 
> > DependentTy anyways, i.e. `1 + ` will have dependent type 
> > with this patch, right?
> > 
> > 
> Using `DependentTy` for `TypoExpr` is a mistake; it leads to all sorts of bad 
> follow-on diagnostics incorrectly claiming that constructs have dependent 
> types.
> 
> Rather than calling this `RecoveryTy`, I'd prefer to call it something a bit 
> more general like `ErrorTy`, with the intent being that we eventually use it 
> for `TypoExpr` too.
> Why are you creating a new type as opposed to just using DependentTy (sorta 
> like TypoExpr does)?

Indeed it's probably redundant in this form of the patch. While I was still 
trying to get this to work in C mode, I was checking for RecoveryTy instead of 
isDependent() in various places. (Maybe that approach would always miss cases, 
as you suggest)

(Obviously if we *don't* reuse the concept of dependent types, then some new 
type/type concepts are needed.)



Comment at: include/clang/AST/Expr.h:5801-5809
+/// RecoveryExpr - a broken expression that we couldn't construct an AST for,
+/// e.g. because overload resolution failed.
+/// We capture:
+///  - the kind of expression that would have been produced
+///  - the valid subexpressions
+///  - the type, if known. If unknown, it is the built-in RecoveryTy, which
+///is dependent (and so generally suppresses further diagnostics etc).

rsmith wrote:
> Do we need this at all, if we have a properly-propagated `ErrorTy` anyway? 
> Instead of using this, it would seem that we could model an ill-formed 
> expression as the corresponding AST node but with its type set to `ErrorTy`. 
> Presumably the latter is what  we'll do when we find a subexpression 
> containing an error, rather than creating a `RecoveryExpr` at every enclosing 
> syntactic level, so AST clients will need to deal with that anyway.
> properly-propagated `ErrorTy`

An important part here (especially for code completion, but not only) is that 
there are RecoveryExprs where the real type is known. So `ErrorTy` doesn't mean 
"subexpression has errors".

The motivating case is:
```
string foo(param1, param2, param3, param4);
foo(param1, param2, param3);
```

Here `foo` has type `string`, not `ErrorTy`.

---

But we can certainly add a bit to Stmt for "subexpression has errors", so the 
question stands:

> as the corresponding AST node but [with the HasError bit set]. Presumably the 
> latter is what we'll do when we find a subexpression containing an error, 
> rather than creating a RecoveryExpr at every enclosing syntactic level, so 
> AST clients will need to deal with that anyway.

Yes. I did try this and we debated it a lot. Somehow I forgot to mention this 
central design question in the patch description :-(

It certainly preserves more information/structure, and

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:1521
   Tok[0].setAnnotationValue(AnnotationVal);
-  EnterTokenStream(std::move(Tok), 1, true);
+  EnterTokenStream(std::move(Tok), 1, true, /*IsReinject*/ true);
 }

rsmith wrote:
> I think it'd be more useful to treat this as a new token. But that's not a 
> strong preference.
Ah, there were too many changes and I missed this one. Definitely agree, 
clients can filter out annotation tokens themselves, if needed.



Comment at: clang/lib/Lex/Pragma.cpp:370
   // Push the tokens onto the stack.
-  EnterTokenStream(TokArray, PragmaToks.size(), true, true);
+  EnterTokenStream(TokArray, PragmaToks.size(), true, true,
+   /*IsReinject*/ false);

rsmith wrote:
> I think this case is a reinjection; we've copied some tokens inside __pragma 
> out into a duplicate position in the token stream. But I guess it doesn't 
> matter because the tokens never escape the outer Lex function anyway.
Yeah, my logic is that it's not a re-injection in the sense that they were 
never the phase 4 tokens before.



Comment at: clang/lib/Lex/Preprocessor.cpp:1136
 EnterTokenStream(std::move(ToksCopy), Toks.size(),
- /*DisableMacroExpansion*/ true);
+ /*DisableMacroExpansion*/ true, IsReinject);
   };

rsmith wrote:
> I think this should always be `false`: the tokens we're producing here have 
> never been emitted at `LexLevel` 0 before.
Ah, totally, we lexed the original tokens in this function...
Fixed, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59885



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

[PATCH] D54334: [AVR] Automatically link CRT and libgcc from the system avr-gcc

[Dylan McKay via Phabricator via cfe-commits]

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

I'm very comfortable with the AVR changes at this point, I am going to go ahead 
and commit the patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54334



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

Re: r360308 - [c++20] Implement P0846R0: allow (ADL-only) calls to template-ids whose

[Stephan Bergmann via cfe-commits]

On 15/05/2019 18:16, Stephan Bergmann wrote:

The below commit started to cause the following failure:


...and which apparently got fixed in the meantime, presumably by 
 
"Make tentative parsing to detect template-argument-lists less aggressive"

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

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Sven Panne via Phabricator via cfe-commits]

svenpanne added a comment.

I think `ninja clang-test-tools` is the right target here, and it is happy with 
the change, which is not very surprising: AFAICT, there is just a single, very 
simple check for `run-clang-tidy.py`. Anyway, my manual testing with and 
without `-quiet` shows no problems, either. Furthermore, we have a patched 
version with the above change in our CI for quite some time, and it works fine. 
And if I read the git history correctly, the output should be identical to the 
one before a fix for interleaved output was landed (that fix added the newlines 
somehow).


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61850



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

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Jonas Toth via Phabricator via cfe-commits]

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

LGTM then, i can commit for you again?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61850



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

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

[Djordje Todorovic via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGDebugInfo.cpp:3885
+  if (ArgNo) {
+auto *PD = dyn_cast(VD);
+ParmCache[PD].reset(D);

aprantl wrote:
> A `dyn_cast` followed by an unconditional use seems strange. I would expect 
> either a `cast` or an `if (PD)` check.
Yes, thanks for this!


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

https://reviews.llvm.org/D58035



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

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro updated this revision to Diff 199788.
djtodoro added a comment.

-Careful use of `dyn_cast`
-Fill the `ParamCache` only in the case of `EnableDebugEntryValues` option


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

https://reviews.llvm.org/D58035

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h


Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -133,6 +133,7 @@
 
   llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
+  llvm::DenseMap ParmCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
   /// using declarations) that aren't covered by other more specific caches.
   llvm::DenseMap DeclCache;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -18,6 +18,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
+#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
@@ -3880,6 +3881,12 @@
  llvm::DebugLoc::get(Line, Column, Scope, 
CurInlinedAt),
  Builder.GetInsertBlock());
 
+  if (CGM.getCodeGenOpts().EnableDebugEntryValues &&
+  CGM.getLangOpts().Optimize && ArgNo) {
+if (auto *PD = dyn_cast(VD))
+  ParmCache[PD].reset(D);
+  }
+
   return D;
 }
 
@@ -4526,6 +4533,25 @@
 if (auto MD = TypeCache[RT])
   DBuilder.retainType(cast(MD));
 
+  if (CGM.getCodeGenOpts().EnableDebugEntryValues &&
+  CGM.getLangOpts().Optimize) {
+for (auto &SP : DeclCache) {
+  auto *D = SP.first;
+  if (const auto *FD = dyn_cast_or_null(D)) {
+const Stmt *FuncBody = (*FD).getBody();
+for (auto Parm : FD->parameters()) {
+  ExprMutationAnalyzer FuncAnalyzer(*FuncBody, CGM.getContext());
+  if (!FuncAnalyzer.isMutated(Parm)) {
+auto I = ParmCache.find(Parm);
+if (I != ParmCache.end()) {
+  auto *DIParm = cast(I->second);
+  DIParm->setIsNotModified();
+}
+  }
+}
+  }
+}
+  }
   DBuilder.finalize();
 }
 


Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -133,6 +133,7 @@
 
   llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
+  llvm::DenseMap ParmCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
   /// using declarations) that aren't covered by other more specific caches.
   llvm::DenseMap DeclCache;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -18,6 +18,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
+#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
@@ -3880,6 +3881,12 @@
  llvm::DebugLoc::get(Line, Column, Scope, CurInlinedAt),
  Builder.GetInsertBlock());
 
+  if (CGM.getCodeGenOpts().EnableDebugEntryValues &&
+  CGM.getLangOpts().Optimize && ArgNo) {
+if (auto *PD = dyn_cast(VD))
+  ParmCache[PD].reset(D);
+  }
+
   return D;
 }
 
@@ -4526,6 +4533,25 @@
 if (auto MD = TypeCache[RT])
   DBuilder.retainType(cast(MD));
 
+  if (CGM.getCodeGenOpts().EnableDebugEntryValues &&
+  CGM.getLangOpts().Optimize) {
+for (auto &SP : DeclCache) {
+  auto *D = SP.first;
+  if (const auto *FD = dyn_cast_or_null(D)) {
+const Stmt *FuncBody = (*FD).getBody();
+for (auto Parm : FD->parameters()) {
+  ExprMutationAnalyzer FuncAnalyzer(*FuncBody, CGM.getContext());
+  if (!FuncAnalyzer.isMutated(Parm)) {
+auto I = ParmCache.find(Parm);
+if (I != ParmCache.end()) {
+  auto *DIParm = cast(I->second);
+  DIParm->setIsNotModified();
+}
+  }
+}
+  }
+}
+  }
   DBuilder.finalize();
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61816: [CFG] [analyzer] pr41300: Add a branch to skip virtual base initializers when they are handled by the superclass.

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

Szelethus added a comment.

Hmmm, how about this?

  #include 
  
  struct VBase1 {
VBase1() { std::cout << "VBase1()\n"; }
VBase1(int) { std::cout << "VBase1(int)\n"; }
  };
  
  struct VBase2 {
VBase2() { std::cout << "VBase2()\n"; }
VBase2(std::string) { std::cout << "VBase2(std::string)\n"; }
  };
  
  struct B : public virtual VBase1 {
B() : VBase1(0) {}
  };
  
  struct C : public B, public virtual VBase2 {
C() : VBase2("sajt") {}
  };
  
  int main() {
C c;
  }

Output:

  VBase1()
  VBase2(std::string)

You are right that all virtual bases as initialized before everything else, but 
not all ctor delegations are skipped. If not this specific one, can something 
similar screw us over?




Comment at: clang/lib/Analysis/CFG.cpp:1434-1438
+  // For C++ constructor add initializers to CFG. Virtual base classes may have
+  // already been initialized by the superclass. Make a branch so that it was
+  // possible to jump straight to non-virtual bases and fields, skipping
+  // virtual bases. We only need to do this once because all virtual base
+  // initializers go all together before all other initializers.

I find this a little too vague. The standard states:

> First, and only for the constructor of the most derived class (4.5) , virtual 
> base classes are initialized in the order they appear on a depth-first 
> left-to-right traversal of the directed acyclic graph of base classes, where 
> “left-to-right” is the order of appearance of the base classes in the derived 
> class base-specifier-list.
>
> Then, direct base classes are initialized in declaration order as they appear 
> in the (regardless of the order of the mem-initializer s).
>
> Then, non-static data members are initialized in the order they were declared 
> in the class definition (again regardless of the order of the mem-initializer 
> s).
>
> Finally, the base-specifier-list compound-statement of the constructor body 
> is executed.

This would be an overkill here, but how does this sound like:

> Constructor delegations are ignored to virtual bases unless the object is of 
> the most derived class.
>
>  class VBase { VBase() = default; VBase(int) {} };
>  class A : virtual public VBase { A() : VBase(0) {} };
>  class B : public A {};
>
>  B b; // Constructor calls in order: VBase(), A(), B().
>   // VBase(int) is ignored, B isn't the most derived class that
>   // delegates to the virtual base.
>
> This may result in the virtual base(s) being already initialized at this 
> point, in which case we should jump right onto non-virtual bases and fields. 
> To handle this, make a CFG branch. We only need to do this once, since the 
> standard states that all virtual bases shall be initialized before 
> non-virtual bases and direct data members.

Also, the comment of mine complementing this inline raises a concern about 
"doing this only once", can you specify?


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

https://reviews.llvm.org/D61816



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

[clang-tools-extra] r360882 - [clang-tidy] Handle member variables in readability-simplify-boolean-expr

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Thu May 16 05:35:00 2019
New Revision: 360882

URL: http://llvm.org/viewvc/llvm-project?rev=360882&view=rev
Log:
[clang-tidy] Handle member variables in readability-simplify-boolean-expr

- Add readability-simplify-boolean-expr test cases for member variables
Fixes PR40179

Patch by LegalizeAdulthood.

Reviewers: alexfh, hokein, aaron.ballman, JonasToth

Reviewed By: JonasToth

Subscribers: jdoerfert, xazax.hun, cfe-commits

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

Added:

clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-members.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp?rev=360882&r1=360881&r2=360882&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp 
Thu May 16 05:35:00 2019
@@ -44,7 +44,7 @@ const char IfAssignVariableId[] = "if-as
 const char IfAssignLocId[] = "if-assign-loc";
 const char IfAssignBoolId[] = "if-assign";
 const char IfAssignNotBoolId[] = "if-assign-not";
-const char IfAssignObjId[] = "if-assign-obj";
+const char IfAssignVarId[] = "if-assign-var";
 const char CompoundReturnId[] = "compound-return";
 const char CompoundBoolId[] = "compound-bool";
 const char CompoundNotBoolId[] = "compound-bool-not";
@@ -360,7 +360,7 @@ void SimplifyBooleanExprCheck::reportBin
   const auto *LHS = Op->getLHS()->IgnoreParenImpCasts();
   const auto *RHS = Op->getRHS()->IgnoreParenImpCasts();
 
-  const CXXBoolLiteralExpr *Bool = nullptr;
+  const CXXBoolLiteralExpr *Bool;
   const Expr *Other = nullptr;
   if ((Bool = dyn_cast(LHS)))
 Other = RHS;
@@ -459,29 +459,28 @@ void SimplifyBooleanExprCheck::matchIfRe
 
 void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder,
   bool Value, StringRef Id) {
-  auto SimpleThen = binaryOperator(
-  hasOperatorName("="),
-  hasLHS(declRefExpr(hasDeclaration(decl().bind(IfAssignObjId,
-  hasLHS(expr().bind(IfAssignVariableId)),
-  hasRHS(cxxBoolLiteral(equals(Value)).bind(IfAssignLocId)));
+  auto VarAssign = declRefExpr(hasDeclaration(decl().bind(IfAssignVarId)));
+  auto VarRef = declRefExpr(hasDeclaration(equalsBoundNode(IfAssignVarId)));
+  auto MemAssign = memberExpr(hasDeclaration(decl().bind(IfAssignVarId)));
+  auto MemRef = memberExpr(hasDeclaration(equalsBoundNode(IfAssignVarId)));
+  auto SimpleThen =
+  binaryOperator(hasOperatorName("="), hasLHS(anyOf(VarAssign, MemAssign)),
+ hasLHS(expr().bind(IfAssignVariableId)),
+ 
hasRHS(cxxBoolLiteral(equals(Value)).bind(IfAssignLocId)));
   auto Then = anyOf(SimpleThen, compoundStmt(statementCountIs(1),
  hasAnySubstatement(SimpleThen)));
-  auto SimpleElse = binaryOperator(
-  hasOperatorName("="),
-  hasLHS(declRefExpr(hasDeclaration(equalsBoundNode(IfAssignObjId,
-  hasRHS(cxxBoolLiteral(equals(!Value;
+  auto SimpleElse =
+  binaryOperator(hasOperatorName("="), hasLHS(anyOf(VarRef, MemRef)),
+ hasRHS(cxxBoolLiteral(equals(!Value;
   auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1),
  hasAnySubstatement(SimpleElse)));
   if (ChainedConditionalAssignment)
+Finder->addMatcher(ifStmt(hasThen(Then), hasElse(Else)).bind(Id), this);
+  else
 Finder->addMatcher(
-ifStmt(isExpansionInMainFile(), hasThen(Then), hasElse(Else)).bind(Id),
+ifStmt(unless(hasParent(ifStmt())), hasThen(Then), hasElse(Else))
+.bind(Id),
 this);
-  else
-Finder->addMatcher(ifStmt(isExpansionInMainFile(),
-  unless(hasParent(ifStmt())), hasThen(Then),
-  hasElse(Else))
-   .bind(Id),
-   this);
 }
 
 void SimplifyBooleanExprCheck::matchCompoundIfReturnsBool(MatchFinder *Finder,

Added: 
clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-members.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-members.cpp?rev=360882&view=auto
==
--- 
clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-members.cpp
 (added)
+++ 
clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-members.cpp
 Thu May 16 05:35:00 2019
@@ -0,0 +1,356 @@
+// RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
+
+class A {
+public:
+  i

[PATCH] D56323: [clang-tidy] Handle member variables in readability-simplify-boolean-expr

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Thank you for the patch and sorry for the long delay.


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

https://reviews.llvm.org/D56323



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

[PATCH] D56323: [clang-tidy] Handle member variables in readability-simplify-boolean-expr

[Jonas Toth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE360882: [clang-tidy] Handle member variables in 
readability-simplify-boolean-expr (authored by JonasToth, committed by ).
Herald added a project: clang.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56323

Files:
  clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  test/clang-tidy/readability-simplify-bool-expr-members.cpp

Index: clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===
--- clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -44,7 +44,7 @@
 const char IfAssignLocId[] = "if-assign-loc";
 const char IfAssignBoolId[] = "if-assign";
 const char IfAssignNotBoolId[] = "if-assign-not";
-const char IfAssignObjId[] = "if-assign-obj";
+const char IfAssignVarId[] = "if-assign-var";
 const char CompoundReturnId[] = "compound-return";
 const char CompoundBoolId[] = "compound-bool";
 const char CompoundNotBoolId[] = "compound-bool-not";
@@ -360,7 +360,7 @@
   const auto *LHS = Op->getLHS()->IgnoreParenImpCasts();
   const auto *RHS = Op->getRHS()->IgnoreParenImpCasts();
 
-  const CXXBoolLiteralExpr *Bool = nullptr;
+  const CXXBoolLiteralExpr *Bool;
   const Expr *Other = nullptr;
   if ((Bool = dyn_cast(LHS)))
 Other = RHS;
@@ -459,29 +459,28 @@
 
 void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder,
   bool Value, StringRef Id) {
-  auto SimpleThen = binaryOperator(
-  hasOperatorName("="),
-  hasLHS(declRefExpr(hasDeclaration(decl().bind(IfAssignObjId,
-  hasLHS(expr().bind(IfAssignVariableId)),
-  hasRHS(cxxBoolLiteral(equals(Value)).bind(IfAssignLocId)));
+  auto VarAssign = declRefExpr(hasDeclaration(decl().bind(IfAssignVarId)));
+  auto VarRef = declRefExpr(hasDeclaration(equalsBoundNode(IfAssignVarId)));
+  auto MemAssign = memberExpr(hasDeclaration(decl().bind(IfAssignVarId)));
+  auto MemRef = memberExpr(hasDeclaration(equalsBoundNode(IfAssignVarId)));
+  auto SimpleThen =
+  binaryOperator(hasOperatorName("="), hasLHS(anyOf(VarAssign, MemAssign)),
+ hasLHS(expr().bind(IfAssignVariableId)),
+ hasRHS(cxxBoolLiteral(equals(Value)).bind(IfAssignLocId)));
   auto Then = anyOf(SimpleThen, compoundStmt(statementCountIs(1),
  hasAnySubstatement(SimpleThen)));
-  auto SimpleElse = binaryOperator(
-  hasOperatorName("="),
-  hasLHS(declRefExpr(hasDeclaration(equalsBoundNode(IfAssignObjId,
-  hasRHS(cxxBoolLiteral(equals(!Value;
+  auto SimpleElse =
+  binaryOperator(hasOperatorName("="), hasLHS(anyOf(VarRef, MemRef)),
+ hasRHS(cxxBoolLiteral(equals(!Value;
   auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1),
  hasAnySubstatement(SimpleElse)));
   if (ChainedConditionalAssignment)
+Finder->addMatcher(ifStmt(hasThen(Then), hasElse(Else)).bind(Id), this);
+  else
 Finder->addMatcher(
-ifStmt(isExpansionInMainFile(), hasThen(Then), hasElse(Else)).bind(Id),
+ifStmt(unless(hasParent(ifStmt())), hasThen(Then), hasElse(Else))
+.bind(Id),
 this);
-  else
-Finder->addMatcher(ifStmt(isExpansionInMainFile(),
-  unless(hasParent(ifStmt())), hasThen(Then),
-  hasElse(Else))
-   .bind(Id),
-   this);
 }
 
 void SimplifyBooleanExprCheck::matchCompoundIfReturnsBool(MatchFinder *Finder,
Index: test/clang-tidy/readability-simplify-bool-expr-members.cpp
===
--- test/clang-tidy/readability-simplify-bool-expr-members.cpp
+++ test/clang-tidy/readability-simplify-bool-expr-members.cpp
@@ -0,0 +1,356 @@
+// RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
+
+class A {
+public:
+  int m;
+};
+
+struct S {
+  S() : m_ar(s_a) {}
+
+  void operator_equals();
+  void operator_or();
+  void operator_and();
+  void ternary_operator();
+  void operator_not_equal();
+  void simple_conditional_assignment_statements();
+  void complex_conditional_assignment_statements();
+  void chained_conditional_assignment();
+  bool non_null_pointer_condition();
+  bool null_pointer_condition();
+  bool negated_non_null_pointer_condition();
+  bool negated_null_pointer_condition();
+  bool integer_not_zero();
+  bool member_pointer_nullptr();
+  bool integer_member_implicit_cast();
+  bool expr_with_cleanups();
+
+  bool m_b1 = false;
+  bool m_b2 = false;
+  bool m_b3 = false;
+  bool m_b4 = false;
+  int *m_p = nullptr;
+  int A::*m_m = nullptr;
+  int m_i = 0;
+  A *m_a = nullptr;
+  static A s_a;
+  A &m_ar;
+};
+
+void S::operator_equals() {
+ 

[clang-tools-extra] r360883 - [clang-tidy] Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Thu May 16 05:39:22 2019
New Revision: 360883

URL: http://llvm.org/viewvc/llvm-project?rev=360883&view=rev
Log:
[clang-tidy] Removed superfluous and slightly annoying newlines in 
run-clang-tidy's output.

Summary: The output of clang-tidy itself already has enough newlines, so the 
resulting output is more in line with the usual compiler output.

Patch by svenpanne.

Reviewers: alexfh, JonasToth

Reviewed By: JonasToth

Subscribers: JonasToth, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py

Modified: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py?rev=360883&r1=360882&r2=360883&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py Thu May 16 
05:39:22 2019
@@ -170,10 +170,10 @@ def run_tidy(args, tmpdir, build_path, q
 if proc.returncode != 0:
   failed_files.append(name)
 with lock:
-  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + 
'\n')
+  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8'))
   if len(err) > 0:
 sys.stdout.flush()
-sys.stderr.write(err.decode('utf-8') + '\n')
+sys.stderr.write(err.decode('utf-8'))
 queue.task_done()
 
 


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

r360885 - Fix isInSystemMacro in presence of macro and pasted token

[Serge Guelton via cfe-commits]

Author: serge_sans_paille
Date: Thu May 16 05:40:00 2019
New Revision: 360885

URL: http://llvm.org/viewvc/llvm-project?rev=360885&view=rev
Log:
Fix isInSystemMacro in presence of macro and pasted token

When a warning is raised from the expansion of a system macro that
involves pasted token, there was still situations were they were not
skipped, as showcased by this issue:
https://bugzilla.redhat.com/show_bug.cgi?id=1472437

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

Modified:
cfe/trunk/include/clang/Basic/SourceManager.h
cfe/trunk/test/Misc/no-warn-in-system-macro.c

Modified: cfe/trunk/include/clang/Basic/SourceManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=360885&r1=360884&r2=360885&view=diff
==
--- cfe/trunk/include/clang/Basic/SourceManager.h (original)
+++ cfe/trunk/include/clang/Basic/SourceManager.h Thu May 16 05:40:00 2019
@@ -1466,8 +1466,13 @@ public:
 
 // This happens when the macro is the result of a paste, in that case
 // its spelling is the scratch memory, so we take the parent context.
-if (isWrittenInScratchSpace(getSpellingLoc(loc)))
-  return isInSystemHeader(getSpellingLoc(getImmediateMacroCallerLoc(loc)));
+// There can be several level of token pasting.
+if (isWrittenInScratchSpace(getSpellingLoc(loc))) {
+  do {
+loc = getImmediateMacroCallerLoc(loc);
+  } while (isWrittenInScratchSpace(getSpellingLoc(loc)));
+  return isInSystemMacro(loc);
+}
 
 return isInSystemHeader(getSpellingLoc(loc));
   }

Modified: cfe/trunk/test/Misc/no-warn-in-system-macro.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/no-warn-in-system-macro.c?rev=360885&r1=360884&r2=360885&view=diff
==
--- cfe/trunk/test/Misc/no-warn-in-system-macro.c (original)
+++ cfe/trunk/test/Misc/no-warn-in-system-macro.c Thu May 16 05:40:00 2019
@@ -3,11 +3,16 @@
 
 #include 
 
+#define MACRO(x) x
+
 int main(void)
 {
double foo = 1.0;
 
if (isnan(foo))
return 1;
-   return 0;
+
+MACRO(isnan(foo));
+
+return 0;
 }


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

[PATCH] D61774: [LibTooling] Add RangeSelector library for defining source ranges based on bound AST nodes.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks! Mostly NITs from me, the design looks nice! Would you mind adding some 
tests before we land this?

The only major thing that I'd argue against is adding helper functions like 
`findPreviousToken` to `SourceCode.h`.
It's fine to have them in the `.cpp` files if they make the implementation 
simpler, but we want to design them more carefully if we put them in the public 
API.




Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:29
+
+namespace range_selector {
+inline RangeSelector charRange(CharSourceRange R) {

Maybe try putting it into the tooling namespace directly?
Or are there immediate and unfortunate conflicts with existing names?





Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:35
+
+/// \returns the range corresponding to the identified node.
+RangeSelector node(StringRef Id);

NIT: maybe explain what Id is?



Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:36
+/// \returns the range corresponding to the identified node.
+RangeSelector node(StringRef Id);
+/// Variant of \c node() that identifies the node as a statement, for purposes

NIT: I believe LLVM naming rules would mandate to name it  `ID`



Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:37
+RangeSelector node(StringRef Id);
+/// Variant of \c node() that identifies the node as a statement, for purposes
+/// of deciding whether to include any trailing semicolon in the selected 
range.

Maybe a shorter description could suffice?
```
Selects a node and a trailing semicolon. Useful for selecting expression 
statements.
```



Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:43
+/// statement.
+RangeSelector sNode(StringRef Id);
+

NIT: maybe name it `statement`?



Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:46
+/// Convenience version of \c range where end points are nodes.
+RangeSelector nodeRange(StringRef BeginId, StringRef EndId);
+

Could this be an overload with the same name?



Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:53
+/// token of the relevant name, not including qualifiers.
+RangeSelector name(StringRef Id);
+

NIT: this is super-specialized, but has a very generic name, therefore might 
cause confusion. Maybe call it `ctorInitializerName`?



Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:57
+// arguments (all source between the call's parentheses).
+RangeSelector args(StringRef Id);
+

Same thing, maybe rename to `callExprArgs`?
And other nodes too



Comment at: clang/include/clang/Tooling/Refactoring/RangeSelector.h:74
+/// `SourceManager::getExpansionRange`.
+RangeSelector contraction(RangeSelector S);
+} // namespace range_selector

NIT: maybe call it `expansion`? 

`contraction` is a new term, might be confusing to use.





Comment at: clang/include/clang/Tooling/Refactoring/SourceCode.h:76
+
+SourceLocation findPreviousTokenStart(SourceLocation Start,
+  const SourceManager &SM,

Not sure this should be a public API. Maybe keep it private to the use-site?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61774



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

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Jonas Toth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL360883: [clang-tidy] Removed superfluous and slightly 
annoying newlines in run-clang… (authored by JonasToth, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61850?vs=199230&id=199800#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61850

Files:
  clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
@@ -170,10 +170,10 @@
 if proc.returncode != 0:
   failed_files.append(name)
 with lock:
-  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + 
'\n')
+  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8'))
   if len(err) > 0:
 sys.stdout.flush()
-sys.stderr.write(err.decode('utf-8') + '\n')
+sys.stderr.write(err.decode('utf-8'))
 queue.task_done()
 
 


Index: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
@@ -170,10 +170,10 @@
 if proc.returncode != 0:
   failed_files.append(name)
 with lock:
-  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + '\n')
+  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8'))
   if len(err) > 0:
 sys.stdout.flush()
-sys.stderr.write(err.decode('utf-8') + '\n')
+sys.stderr.write(err.decode('utf-8'))
 queue.task_done()
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59413: Fix isInSystemMacro in presence of macro and pasted token

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC360885: Fix isInSystemMacro in presence of macro and pasted 
token (authored by serge_sans_paille, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59413?vs=199585&id=199801#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59413

Files:
  include/clang/Basic/SourceManager.h
  test/Misc/no-warn-in-system-macro.c


Index: include/clang/Basic/SourceManager.h
===
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h
@@ -1466,8 +1466,13 @@
 
 // This happens when the macro is the result of a paste, in that case
 // its spelling is the scratch memory, so we take the parent context.
-if (isWrittenInScratchSpace(getSpellingLoc(loc)))
-  return isInSystemHeader(getSpellingLoc(getImmediateMacroCallerLoc(loc)));
+// There can be several level of token pasting.
+if (isWrittenInScratchSpace(getSpellingLoc(loc))) {
+  do {
+loc = getImmediateMacroCallerLoc(loc);
+  } while (isWrittenInScratchSpace(getSpellingLoc(loc)));
+  return isInSystemMacro(loc);
+}
 
 return isInSystemHeader(getSpellingLoc(loc));
   }
Index: test/Misc/no-warn-in-system-macro.c
===
--- test/Misc/no-warn-in-system-macro.c
+++ test/Misc/no-warn-in-system-macro.c
@@ -3,11 +3,16 @@
 
 #include 
 
+#define MACRO(x) x
+
 int main(void)
 {
double foo = 1.0;
 
if (isnan(foo))
return 1;
-   return 0;
+
+MACRO(isnan(foo));
+
+return 0;
 }


Index: include/clang/Basic/SourceManager.h
===
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h
@@ -1466,8 +1466,13 @@
 
 // This happens when the macro is the result of a paste, in that case
 // its spelling is the scratch memory, so we take the parent context.
-if (isWrittenInScratchSpace(getSpellingLoc(loc)))
-  return isInSystemHeader(getSpellingLoc(getImmediateMacroCallerLoc(loc)));
+// There can be several level of token pasting.
+if (isWrittenInScratchSpace(getSpellingLoc(loc))) {
+  do {
+loc = getImmediateMacroCallerLoc(loc);
+  } while (isWrittenInScratchSpace(getSpellingLoc(loc)));
+  return isInSystemMacro(loc);
+}
 
 return isInSystemHeader(getSpellingLoc(loc));
   }
Index: test/Misc/no-warn-in-system-macro.c
===
--- test/Misc/no-warn-in-system-macro.c
+++ test/Misc/no-warn-in-system-macro.c
@@ -3,11 +3,16 @@
 
 #include 
 
+#define MACRO(x) x
+
 int main(void)
 {
 	double foo = 1.0;
 
 	if (isnan(foo))
 		return 1;
-	return 0;
+
+MACRO(isnan(foo));
+
+return 0;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62005: [libunwind] [test] Fix inferring source paths

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

mgorny created this revision.
mgorny added reviewers: jroelofs, krytarowski.
Herald added subscribers: libcxx-commits, ldionne, christof.

Fix two issues that caused libcxx source path not to be inferred
correctly when not specified explicitly:

1. get_lit_conf() uses default value only if the lit variable is set to None.  
Due to the mehod of substituting lit.site.cfg, they were "" rather than None 
when unset, effectively causing the default never to apply.  Instead, use 'or' 
construct to use the default whenever get_lit_conf() returns a false value.

2. If os.path.join() is given a component starting with '/', it takes it to be 
an absolute path and ignores everything preceding it. Remove the slash to 
correctly append subdirectory.

With these two fixes, libunwind tests start working on NetBSD buildbot
again.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D62005

Files:
  libunwind/test/libunwind/test/config.py


Index: libunwind/test/libunwind/test/config.py
===
--- libunwind/test/libunwind/test/config.py
+++ libunwind/test/libunwind/test/config.py
@@ -21,12 +21,10 @@
 self.libcxx_src_root = None
 
 def configure_src_root(self):
-self.libunwind_src_root = self.get_lit_conf(
-'libunwind_src_root',
-os.path.dirname(self.config.test_source_root))
-self.libcxx_src_root = self.get_lit_conf(
-'libcxx_src_root',
-os.path.join(self.libunwind_src_root, '/../libcxx'))
+self.libunwind_src_root = (self.get_lit_conf('libunwind_src_root')
+or os.path.dirname(self.config.test_source_root))
+self.libcxx_src_root = (self.get_lit_conf('libcxx_src_root')
+or os.path.join(self.libunwind_src_root, '../libcxx'))
 
 def configure_obj_root(self):
 self.libunwind_obj_root = self.get_lit_conf('libunwind_obj_root')


Index: libunwind/test/libunwind/test/config.py
===
--- libunwind/test/libunwind/test/config.py
+++ libunwind/test/libunwind/test/config.py
@@ -21,12 +21,10 @@
 self.libcxx_src_root = None
 
 def configure_src_root(self):
-self.libunwind_src_root = self.get_lit_conf(
-'libunwind_src_root',
-os.path.dirname(self.config.test_source_root))
-self.libcxx_src_root = self.get_lit_conf(
-'libcxx_src_root',
-os.path.join(self.libunwind_src_root, '/../libcxx'))
+self.libunwind_src_root = (self.get_lit_conf('libunwind_src_root')
+or os.path.dirname(self.config.test_source_root))
+self.libcxx_src_root = (self.get_lit_conf('libcxx_src_root')
+or os.path.join(self.libunwind_src_root, '../libcxx'))
 
 def configure_obj_root(self):
 self.libunwind_obj_root = self.get_lit_conf('libunwind_obj_root')
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61850: Removed superfluous and slightly annoying newlines in run-clang-tidy's output.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Thans for the patch!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61850



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

[PATCH] D61835: Extract ASTDumper to a header file

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I'm not certain where you're planning to go with this change (or is this the 
only change you're trying to make in this area?), so it's a bit hard to 
evaluate this patch. Can you explain a bit more about what you're ultimately 
trying to accomplish?

It might help if I had a better idea of which APIs you thought were ones that 
would help users (because my only real concern with this change is that the 
public interface for this class is rather unpleasant).


Repository:
  rC Clang

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

https://reviews.llvm.org/D61835



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

[PATCH] D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

What will be making use of/testing this new functionality?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61834



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

r360891 - Reland "[analyzer] Add an example plugin for checker dependency handling"

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Thu May 16 06:22:04 2019
New Revision: 360891

URL: http://llvm.org/viewvc/llvm-project?rev=360891&view=rev
Log:
Reland "[analyzer] Add an example plugin for checker dependency handling"

Buildbots complained that they couldn't find the newly added plugins.

The solution was to move the check-clang cmake target closer to the bottom of
the file, after the new dependencies are added.

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

Added:
cfe/trunk/test/Analysis/plugins/
  - copied from r360804, cfe/trunk/test/Analysis/plugins/
Removed:
cfe/trunk/examples/analyzer-plugin/
Modified:
cfe/trunk/examples/CMakeLists.txt
cfe/trunk/test/Analysis/checker-plugins.c
cfe/trunk/test/Analysis/lit.local.cfg
cfe/trunk/test/CMakeLists.txt

Modified: cfe/trunk/examples/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/examples/CMakeLists.txt?rev=360891&r1=360890&r2=360891&view=diff
==
--- cfe/trunk/examples/CMakeLists.txt (original)
+++ cfe/trunk/examples/CMakeLists.txt Thu May 16 06:22:04 2019
@@ -3,9 +3,6 @@ if(NOT CLANG_BUILD_EXAMPLES)
   set(EXCLUDE_FROM_ALL ON)
 endif()
 
-if(CLANG_ENABLE_STATIC_ANALYZER)
-add_subdirectory(analyzer-plugin)
-endif()
 add_subdirectory(clang-interpreter)
 add_subdirectory(PrintFunctionNames)
 add_subdirectory(AnnotateFunctions)

Modified: cfe/trunk/test/Analysis/checker-plugins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/checker-plugins.c?rev=360891&r1=360890&r2=360891&view=diff
==
--- cfe/trunk/test/Analysis/checker-plugins.c (original)
+++ cfe/trunk/test/Analysis/checker-plugins.c Thu May 16 06:22:04 2019
@@ -1,5 +1,8 @@
-// RUN: %clang_analyze_cc1 -load %llvmshlibdir/SampleAnalyzerPlugin%pluginext 
-analyzer-checker='example.MainCallChecker' -verify %s
-// REQUIRES: plugins, examples
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -load %llvmshlibdir/SampleAnalyzerPlugin%pluginext \
+// RUN:   -analyzer-checker='example.MainCallChecker'
+
+// REQUIRES: plugins
 
 // Test that the MainCallChecker example analyzer plugin loads and runs.
 
@@ -8,3 +11,22 @@ int main();
 void caller() {
   main(); // expected-warning {{call to main}}
 }
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load 
%llvmshlibdir/CheckerDependencyHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.DependendentChecker \
+// RUN:   -analyzer-list-enabled-checkers \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-ENABLED
+
+// CHECK-IMPLICITLY-ENABLED: example.Dependency
+// CHECK-IMPLICITLY-ENABLED: example.DependendentChecker
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load 
%llvmshlibdir/CheckerDependencyHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.DependendentChecker \
+// RUN:   -analyzer-disable-checker=example.Dependency \
+// RUN:   -analyzer-list-enabled-checkers \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-DISABLED
+
+// CHECK-IMPLICITLY-DISABLED-NOT: example.Dependency
+// CHECK-IMPLICITLY-DISABLED-NOT: example.DependendentChecker

Modified: cfe/trunk/test/Analysis/lit.local.cfg
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/lit.local.cfg?rev=360891&r1=360890&r2=360891&view=diff
==
--- cfe/trunk/test/Analysis/lit.local.cfg (original)
+++ cfe/trunk/test/Analysis/lit.local.cfg Thu May 16 06:22:04 2019
@@ -18,5 +18,7 @@ config.substitutions.append(('%diff_plis
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))
 
+config.excludes.add('plugins')
+
 if not config.root.clang_staticanalyzer:
 config.unsupported = True

Modified: cfe/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CMakeLists.txt?rev=360891&r1=360890&r2=360891&view=diff
==
--- cfe/trunk/test/CMakeLists.txt (original)
+++ cfe/trunk/test/CMakeLists.txt Thu May 16 06:22:04 2019
@@ -123,19 +123,10 @@ if( NOT CLANG_BUILT_STANDALONE )
   endif()
 endif()
 
-add_custom_target(clang-test-depends DEPENDS ${CLANG_TEST_DEPS})
-set_target_properties(clang-test-depends PROPERTIES FOLDER "Clang tests")
-
-add_lit_testsuite(check-clang "Running the Clang regression tests"
-  ${CMAKE_CURRENT_BINARY_DIR}
-  #LIT ${LLVM_LIT}
-  PARAMS ${CLANG_TEST_PARAMS}
-  DEPENDS ${CLANG_TEST_DEPS}
-  ARGS ${CLANG_TEST_EXTRA_ARGS}
-  )
-set_target_properties(check-clang PROPERTIES FOLDER "Clang tests")
-
 if (CLANG_ENABLE_STATIC_ANALYZER)
+  add_subdirectory(Analysis/plugins)
+  list(APPEND CLANG_TEST_DEPS clang-analyzer-plugin)
+
   # check-all would launch those tests via check-clang.
   set(EXCLUDE_FROM_ALL ON)
 
@@ -145,7 +136,6 @@ if (CLANG_ENABLE_

[PATCH] D59464: [analyzer] Add an example plugin for checker dependency handling

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL360891: Reland "[analyzer] Add an example plugin for 
checker dependency handling" (authored by Szelethus, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59464?vs=199663&id=199807#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59464

Files:
  cfe/trunk/examples/CMakeLists.txt
  cfe/trunk/examples/analyzer-plugin/CMakeLists.txt
  cfe/trunk/examples/analyzer-plugin/MainCallChecker.cpp
  cfe/trunk/examples/analyzer-plugin/SampleAnalyzerPlugin.exports
  cfe/trunk/test/Analysis/checker-plugins.c
  cfe/trunk/test/Analysis/lit.local.cfg
  cfe/trunk/test/Analysis/plugins/CMakeLists.txt
  cfe/trunk/test/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
  
cfe/trunk/test/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandling.cpp
  
cfe/trunk/test/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandlingAnalyzerPlugin.exports
  cfe/trunk/test/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
  cfe/trunk/test/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
  cfe/trunk/test/Analysis/plugins/SampleAnalyzer/SampleAnalyzerPlugin.exports
  cfe/trunk/test/CMakeLists.txt

Index: cfe/trunk/test/CMakeLists.txt
===
--- cfe/trunk/test/CMakeLists.txt
+++ cfe/trunk/test/CMakeLists.txt
@@ -123,19 +123,10 @@
   endif()
 endif()
 
-add_custom_target(clang-test-depends DEPENDS ${CLANG_TEST_DEPS})
-set_target_properties(clang-test-depends PROPERTIES FOLDER "Clang tests")
-
-add_lit_testsuite(check-clang "Running the Clang regression tests"
-  ${CMAKE_CURRENT_BINARY_DIR}
-  #LIT ${LLVM_LIT}
-  PARAMS ${CLANG_TEST_PARAMS}
-  DEPENDS ${CLANG_TEST_DEPS}
-  ARGS ${CLANG_TEST_EXTRA_ARGS}
-  )
-set_target_properties(check-clang PROPERTIES FOLDER "Clang tests")
-
 if (CLANG_ENABLE_STATIC_ANALYZER)
+  add_subdirectory(Analysis/plugins)
+  list(APPEND CLANG_TEST_DEPS clang-analyzer-plugin)
+
   # check-all would launch those tests via check-clang.
   set(EXCLUDE_FROM_ALL ON)
 
@@ -145,7 +136,6 @@
 DEPENDS ${CLANG_TEST_DEPS})
   set_target_properties(check-clang-analyzer PROPERTIES FOLDER "Clang tests")
 
-
   if (LLVM_WITH_Z3)
 add_lit_testsuite(check-clang-analyzer-z3 "Running the Clang analyzer tests, using Z3 as a solver"
   ${CMAKE_CURRENT_BINARY_DIR}/Analysis
@@ -157,6 +147,18 @@
   set(EXCLUDE_FROM_ALL OFF)
 endif()
 
+add_custom_target(clang-test-depends DEPENDS ${CLANG_TEST_DEPS})
+set_target_properties(clang-test-depends PROPERTIES FOLDER "Clang tests")
+
+add_lit_testsuite(check-clang "Running the Clang regression tests"
+  ${CMAKE_CURRENT_BINARY_DIR}
+  #LIT ${LLVM_LIT}
+  PARAMS ${CLANG_TEST_PARAMS}
+  DEPENDS ${CLANG_TEST_DEPS}
+  ARGS ${CLANG_TEST_EXTRA_ARGS}
+  )
+set_target_properties(check-clang PROPERTIES FOLDER "Clang tests")
+
 add_lit_testsuites(CLANG ${CMAKE_CURRENT_SOURCE_DIR}
   PARAMS ${CLANG_TEST_PARAMS}
   DEPENDS ${CLANG_TEST_DEPS}
Index: cfe/trunk/test/Analysis/lit.local.cfg
===
--- cfe/trunk/test/Analysis/lit.local.cfg
+++ cfe/trunk/test/Analysis/lit.local.cfg
@@ -18,5 +18,7 @@
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I "2\.0\.0\-csd\.[0-9]*\.beta\."'''))
 
+config.excludes.add('plugins')
+
 if not config.root.clang_staticanalyzer:
 config.unsupported = True
Index: cfe/trunk/test/Analysis/checker-plugins.c
===
--- cfe/trunk/test/Analysis/checker-plugins.c
+++ cfe/trunk/test/Analysis/checker-plugins.c
@@ -1,5 +1,8 @@
-// RUN: %clang_analyze_cc1 -load %llvmshlibdir/SampleAnalyzerPlugin%pluginext -analyzer-checker='example.MainCallChecker' -verify %s
-// REQUIRES: plugins, examples
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -load %llvmshlibdir/SampleAnalyzerPlugin%pluginext \
+// RUN:   -analyzer-checker='example.MainCallChecker'
+
+// REQUIRES: plugins
 
 // Test that the MainCallChecker example analyzer plugin loads and runs.
 
@@ -8,3 +11,22 @@
 void caller() {
   main(); // expected-warning {{call to main}}
 }
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load %llvmshlibdir/CheckerDependencyHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.DependendentChecker \
+// RUN:   -analyzer-list-enabled-checkers \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-ENABLED
+
+// CHECK-IMPLICITLY-ENABLED: example.Dependency
+// CHECK-IMPLICITLY-ENABLED: example.DependendentChecker
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load %llvmshlibdir/CheckerDependencyHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.DependendentChecker \
+// RUN:   -analyzer-disable-checker=example.Dependency \
+// RUN:   -analyzer-list-enabled-checkers \
+// RUN:   2>&1 | FileCheck %s -check-pref

r360892 - Recommit [Object] Change object::SectionRef::getContents() to return Expected

[Fangrui Song via cfe-commits]

Author: maskray
Date: Thu May 16 06:24:04 2019
New Revision: 360892

URL: http://llvm.org/viewvc/llvm-project?rev=360892&view=rev
Log:
Recommit [Object] Change object::SectionRef::getContents() to return 
Expected

r360876 didn't fix 2 call sites in clang.

Expected> may be better but use Expected for now.

Follow-up of D61781.

Modified:
cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Modified: cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=360892&r1=360891&r2=360892&view=diff
==
--- cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp (original)
+++ cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp Thu May 16 
06:24:04 2019
@@ -337,8 +337,14 @@ ObjectFilePCHContainerReader::ExtractPCH
   StringRef Name;
   Section.getName(Name);
   if ((!IsCOFF && Name == "__clangast") || (IsCOFF && Name == "clangast")) 
{
-Section.getContents(PCH);
-return PCH;
+if (Expected E = Section.getContents())
+  return *E;
+else {
+  handleAllErrors(E.takeError(), [&](const llvm::ErrorInfoBase &EIB) {
+EIB.log(llvm::errs());
+  });
+  return "";
+}
   }
 }
   }

Modified: cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp?rev=360892&r1=360891&r2=360892&view=diff
==
--- cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp (original)
+++ cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp Thu May 16 
06:24:04 2019
@@ -462,13 +462,16 @@ public:
 // TODO: Instead of copying the input file as is, deactivate the section
 // that is no longer needed.
 
-StringRef Content;
-CurrentSection->getContents(Content);
+Expected Content = CurrentSection->getContents();
+if (!Content) {
+  consumeError(Content.takeError());
+  return;
+}
 
-if (Content.size() < 2)
+if (Content->size() < 2)
   OS.write(Input.getBufferStart(), Input.getBufferSize());
 else
-  OS.write(Content.data(), Content.size());
+  OS.write(Content->data(), Content->size());
   }
 
   void WriteHeader(raw_fd_ostream &OS,


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

[PATCH] D61950: [PowerPC64] adds ABI parsing when specified on target triple

[Alfredo Dal'Ava Júnior via Phabricator via cfe-commits]

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

In D61950#1504169 , @MaskRay wrote:

> Will FreeBSD 13 or future releases support ELFv1? If not, it may be cleaner 
> to not invent `-elfv2` `-elfv1` triples, but rather dispatch on the major 
> version, e.g. `powerpc64-unknown-freebsd13.0` could mean ELFv2.


Invent  `-elfv2` `-elfv1` and make it used by default is not the goal here but 
add ABI target triple support. Discussions at #llvm 
 IRC channel shown that provide ABI in 
target triple is more acceptable than using `-mabi=elfv2`, so this patch is for 
when you want and ABI other than default (that was based on ,  or 
)

For FreeBSD, in the future default ABI will be decided based on OS version on 
triple  `powerpc64-unknown-freebsd`. However, there's currently no 
agreement on what will be that . As 13.0 is currently ELFv1, so 
LLVM couldn't assume 13.0 is ELFv2 at this moment.

So, during the transition period, developers could use TARGET_ABI variable to 
build the FreeBSD PPC64 ELFv2 version. As example,  building FreeBSD with `make 
TARGET=powerpc TARGET_ARCH=powerpc64 TARGET_ABI=elfv2`, where it's expected to 
form a target triple like `powerpc64-unknown-freebsd13.0-elfv2`, based on 
"---" definition. That is passed right away to 
clang.

If it's not possible, we would have to patch FreeBSD makefiles to not use 
TARGET_ABI variable value when TARGET_ARCH is powerpc64, and append -mabi=elfv2 
instead. This sounds feasible, however clang running on final ELFv2 userland 
won't know it's on ELFv2 and users will need to specify `-mabi=elfv2` and 
CFLAGS/CXXFLAGS to compile applications. This is unfeasible, specially for 
FreeBSD ports, as many packages didn't handle these variables correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61950



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

r360896 - [clang] Handle lround/llround builtins

[Adhemerval Zanella via cfe-commits]

Author: azanella
Date: Thu May 16 06:43:25 2019
New Revision: 360896

URL: http://llvm.org/viewvc/llvm-project?rev=360896&view=rev
Log:
[clang] Handle lround/llround builtins

As for other floating-point rounding builtins that can be optimized
when build with -fno-math-errno, this patch adds support for lround
and llround.  It currently only optimize for AArch64 backend.

Reviewed By: efriedma

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


Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/builtins.c
cfe/trunk/test/CodeGen/math-builtins.c
cfe/trunk/test/CodeGen/math-libcalls.c

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=360896&r1=360895&r2=360896&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu May 16 06:43:25 2019
@@ -1721,6 +1721,27 @@ RValue CodeGenFunction::EmitBuiltinExpr(
 case Builtin::BI__builtin_truncl:
   return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::trunc));
 
+case Builtin::BIlround:
+case Builtin::BIlroundf:
+case Builtin::BIlroundl:
+case Builtin::BI__builtin_lround:
+case Builtin::BI__builtin_lroundf:
+case Builtin::BI__builtin_lroundl: {
+  llvm::Type *ResultType = ConvertType(E->getType());
+  int Width = ResultType->getPrimitiveSizeInBits();
+  return RValue::get(emitUnaryBuiltin(*this, E,
+  Width == 32 ? Intrinsic::lround_i32
+  : 
Intrinsic::lround_i64));
+}
+
+case Builtin::BIllround:
+case Builtin::BIllroundf:
+case Builtin::BIllroundl:
+case Builtin::BI__builtin_llround:
+case Builtin::BI__builtin_llroundf:
+case Builtin::BI__builtin_llroundl:
+  return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::llround));
+
 default:
   break;
 }

Modified: cfe/trunk/test/CodeGen/builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins.c?rev=360896&r1=360895&r2=360896&view=diff
==
--- cfe/trunk/test/CodeGen/builtins.c (original)
+++ cfe/trunk/test/CodeGen/builtins.c Thu May 16 06:43:25 2019
@@ -256,6 +256,8 @@ void test_float_builtin_ops(float F, dou
   volatile float resf;
   volatile double resd;
   volatile long double resld;
+  volatile long int resli;
+  volatile long long int reslli;
 
   resf = __builtin_fmodf(F,F);
   // CHECK: frem float
@@ -380,6 +382,14 @@ void test_float_builtin_ops(float F, dou
   resld = __builtin_roundl(LD);
   // CHECK: call x86_fp80 @llvm.round.f80
 
+  resli = __builtin_lroundf (F);
+  // CHECK: call i64 @llvm.lround.i64.f32
+
+  resli = __builtin_lround (D);
+  // CHECK: call i64 @llvm.lround.i64.f64
+
+  resli = __builtin_lroundl (LD);
+  // CHECK: call i64 @llvm.lround.i64.f80
 }
 
 // __builtin_longjmp isn't supported on all platforms, so only test it on X86.

Modified: cfe/trunk/test/CodeGen/math-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/math-builtins.c?rev=360896&r1=360895&r2=360896&view=diff
==
--- cfe/trunk/test/CodeGen/math-builtins.c (original)
+++ cfe/trunk/test/CodeGen/math-builtins.c Thu May 16 06:43:25 2019
@@ -362,9 +362,9 @@ void foo(double *d, float f, float *fp,
 
   __builtin_llround(f);__builtin_llroundf(f);   __builtin_llroundl(f);
 
-// NO__ERRNO: declare i64 @llround(double) [[READNONE]]
-// NO__ERRNO: declare i64 @llroundf(float) [[READNONE]]
-// NO__ERRNO: declare i64 @llroundl(x86_fp80) [[READNONE]]
+// NO__ERRNO: declare i64 @llvm.llround.f64(double) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare i64 @llvm.llround.f32(float) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare i64 @llvm.llround.f80(x86_fp80) [[READNONE_INTRINSIC]]
 // HAS_ERRNO: declare i64 @llround(double) [[NOT_READNONE]]
 // HAS_ERRNO: declare i64 @llroundf(float) [[NOT_READNONE]]
 // HAS_ERRNO: declare i64 @llroundl(x86_fp80) [[NOT_READNONE]]
@@ -425,9 +425,9 @@ void foo(double *d, float f, float *fp,
 
   __builtin_lround(f); __builtin_lroundf(f);__builtin_lroundl(f);
 
-// NO__ERRNO: declare i64 @lround(double) [[READNONE]]
-// NO__ERRNO: declare i64 @lroundf(float) [[READNONE]]
-// NO__ERRNO: declare i64 @lroundl(x86_fp80) [[READNONE]]
+// NO__ERRNO: declare i64 @llvm.lround.i64.f64(double) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare i64 @llvm.lround.i64.f32(float) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare i64 @llvm.lround.i64.f80(x86_fp80) [[READNONE_INTRINSIC]]
 // HAS_ERRNO: declare i64 @lround(double) [[NOT_READNONE]]
 // HAS_ERRNO: declare i64 @lroundf(float) [[NOT_READNONE]]
 // HAS_ERRNO: declare i64 @lroundl(x86_fp80) [[NOT_READNONE]]

Modified: cfe/trunk/test/CodeGen/math-libcalls.

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

[Borsik Gábor via Phabricator via cfe-commits]

boga95 added a comment.

Sorry for the late answer, I was working on my thesis which is about taint 
analysis. During that, I implemented several cool features (namespaces, 
std::cin, std::string, etc.) for the checker. I will share them soon.

I thought about the API notes and I think it will fit very well into the 
checker. If my understanding is clear, the checker would be configured with 
attributes and/or a yaml file which contains the attributes. Therefore, the 
implementation will become simpler, because the checker will only read the 
attributes. I made a draft for the possible usage of the attributes:

  [[taint::dst(-1)]]
  int mySource(); // The return value will become tainted
  
  [[taint::src(0, 1)]] [[taint::dst(2)]]
  void myPropagator(int*, int*, int*);
  
  [[taint::src(0)]] [[taint::varDst(2)]]
  int myFscanf(FILE*, const char*, ...); // The variadic arguments will become 
tainted, if the first argument is tainted
  
  [[taint::dst(0, -1)]] [[taint::varSrc(2)]]
  int mySprintf(char*, const char*, ... ); // The first argument and the return 
value will become tainted, if any of the variadic arguments is tainted

I think we can use the current yaml configuration in order to not block my 
progress. I think most of the current implementation can be reused for the API 
notes. I will be able to easily change the interface after the API notes are 
ready.




Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:73-75
   static const unsigned InvalidArgIndex = UINT_MAX;
   /// Denotes the return vale.
   static const unsigned ReturnValueIndex = UINT_MAX - 1;

Szelethus wrote:
> boga95 wrote:
> > Szelethus wrote:
> > > We should definitely change these, not only is the large integer number 
> > > impossible to remember, but this value could differ on different 
> > > platforms.
> > I tried to use int, but I got a lot of warnings because of the 
> > `getNumArgs()` returns an unsigned value.
> What warnings? I thought we have `-Wsign-conversion` disabled.
I got `-Wsign-compare` warnings, but it compiles. I will change it in the next 
[[ https://reviews.llvm.org/D59637 | review ]] because that's contains the yaml 
file and the related tests.


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

https://reviews.llvm.org/D59555



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

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Given this revision is accepted and seems ready to go: Do you need someone to 
commit on your behalf?


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

https://reviews.llvm.org/D55793



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

[PATCH] D59879: [ARM][CMSE] Add commandline option and feature macro

[Javed Absar via Phabricator via cfe-commits]

javed.absar marked 4 inline comments as done.
javed.absar added inline comments.



Comment at: lib/Basic/Targets/ARM.cpp:438
+} else if (Feature == "+8msecext") {
+  if ((CPUProfile != "M" && CPUProfile != "B") || ArchVersion != 8) {
+Diags.Report(diag::err_target_unsupported_mcmse) << CPU;

snidertm wrote:
> dmgreen wrote:
> > snidertm wrote:
> > > How does CPUProfile get a value of "B"? I thought any Cortex-M processor 
> > > would set CPUProfile to "M". Is CMSE available on a processor besides 
> > > Cortex-m33?
> > "B" was going to be a very old name for v8m-baseline, looks like this one 
> > was never cleaned up when that was changed. You can drop the != "B" check.
> Do all v8 profile == 'M' processor variants support security extensions?
Yes


Repository:
  rC Clang

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

https://reviews.llvm.org/D59879



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

[PATCH] D59879: [ARM][CMSE] Add commandline option and feature macro

[Javed Absar via Phabricator via cfe-commits]

javed.absar updated this revision to Diff 199819.
javed.absar added a comment.

Updated based on review comments


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

https://reviews.llvm.org/D59879

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  lib/Basic/Targets/ARM.cpp
  lib/Driver/ToolChains/Arch/ARM.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Preprocessor/arm-target-features.c

Index: test/Preprocessor/arm-target-features.c
===
--- test/Preprocessor/arm-target-features.c
+++ test/Preprocessor/arm-target-features.c
@@ -198,6 +198,7 @@
 // V8M_BASELINE: #define __ARM_ARCH_ISA_THUMB 1
 // V8M_BASELINE: #define __ARM_ARCH_PROFILE 'M'
 // V8M_BASELINE-NOT: __ARM_FEATURE_CRC32
+// V8M_BASELINE: #define __ARM_FEATURE_CMSE 1
 // V8M_BASELINE-NOT: __ARM_FEATURE_DSP
 // V8M_BASELINE-NOT: __ARM_FP 0x{{.*}}
 // V8M_BASELINE-NOT: __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1
@@ -210,6 +211,7 @@
 // V8M_MAINLINE: #define __ARM_ARCH_ISA_THUMB 2
 // V8M_MAINLINE: #define __ARM_ARCH_PROFILE 'M'
 // V8M_MAINLINE-NOT: __ARM_FEATURE_CRC32
+// V8M_MAINLINE: #define __ARM_FEATURE_CMSE 1
 // V8M_MAINLINE-NOT: __ARM_FEATURE_DSP
 // V8M_MAINLINE-NOT: #define __ARM_FP 0x
 // V8M_MAINLINE: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
@@ -675,6 +677,24 @@
 // M7-THUMB-ALLOW-FP-INSTR:#define __ARM_FP 0xe
 // M7-THUMB-ALLOW-FP-INSTR:#define __ARM_FPV5__ 1
 
+// Check that -mcmse (security extension) option works correctly for v8-M targets
+// RUN: %clang -target armv8m.base-none-linux-gnu -mcmse -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=V8M_CMSE %s
+// RUN: %clang -target armv8m.main-none-linux-gnu -mcmse -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=V8M_CMSE %s
+// RUN: %clang -target arm-none-linux-gnu -mcpu=cortex-m33 -mcmse -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=V8M_CMSE %s
+// RUN: %clang -target arm -mcpu=cortex-m23 -mcmse -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=V8M_CMSE %s
+// V8M_CMSE-NOT: __ARM_FEATURE_CMSE 1
+// V8M_CMSE: #define __ARM_FEATURE_CMSE 3
+
+// Check that CMSE is not defined on architectures w/o support for security extension
+// RUN: %clang -target arm-arm-none-gnueabi -mcpu=cortex-a5 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=NOTV8M_CMSE %s
+// RUN: %clang -target armv8a-none-linux-gnu -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=NOTV8M_CMSE %s
+// RUN: %clang -target armv8.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=NOTV8M_CMSE %s
+// NOTV8M_CMSE-NOT: __ARM_FEATURE_CMSE
+
+// Check that -mcmse option gives error on non v8-M targets
+// RUN: not %clang -target arm-arm-none-eabi -mthumb -mcmse -mcpu=cortex-m7 -x c -E -dM %s -o - 2>&1 | FileCheck -match-full-lines --check-prefix=NOTV8MCMSE_OPT %s
+// NOTV8MCMSE_OPT: error: -mcmse is not supported for cortex-m7
+
 // Test whether predefines are as expected when targeting v8m cores
 // RUN: %clang -target arm -mcpu=cortex-m23 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=M23 %s
 // M23: #define __ARM_ARCH 8
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2729,6 +2729,7 @@
   Opts.HalfArgsAndReturns = Args.hasArg(OPT_fallow_half_arguments_and_returns)
 | Opts.NativeHalfArgsAndReturns;
   Opts.GNUAsm = !Args.hasArg(OPT_fno_gnu_inline_asm);
+  Opts.Cmse = Args.hasArg(OPT_mcmse); // Armv8-M Security Extensions
 
   // __declspec is enabled by default for the PS4 by the driver, and also
   // enabled for Microsoft Extensions or Borland Extensions, here.
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1423,6 +1423,9 @@
   if (!Args.hasFlag(options::OPT_mimplicit_float,
 options::OPT_mno_implicit_float, true))
 CmdArgs.push_back("-no-implicit-float");
+
+  if (Args.getLastArg(options::OPT_mcmse))
+CmdArgs.push_back("-mcmse");
 }
 
 void Clang::RenderTargetOptions(const llvm::Triple &EffectiveTriple,
Index: lib/Driver/ToolChains/Arch/ARM.cpp
===
--- lib/Driver/ToolChains/Arch/ARM.cpp
+++ lib/Driver/ToolChains/Arch/ARM.cpp
@@ -469,6 +469,10 @@
 }
   }
 
+  // CMSE: Check for target 8M (for -mcmse to be applicable) is performed later.
+  if (Args.getLastArg(options::OPT_mcmse))
+Features.push_back("+8msecext");
+
   // Look for the last occurrence of -mlong-calls or -mno-long-calls. If
   // neither options are specified, see if we are compiling for kernel/kext and
 

[PATCH] D61530: Add AIX Version Macros

[Xing Xue via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC360900: Add AIX Version Macros (authored by xingxue, 
committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61530?vs=198056&id=199825#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D61530

Files:
  lib/Basic/Targets/OSTargets.h
  test/Preprocessor/init.c

Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -7243,6 +7243,129 @@
 // PPC-AIX:#define __powerpc__ 1
 // PPC-AIX:#define __ppc__ 1
 //
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.2.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX72 %s
+//
+// PPC-AIX72:#define _AIX32 1
+// PPC-AIX72:#define _AIX41 1
+// PPC-AIX72:#define _AIX43 1
+// PPC-AIX72:#define _AIX50 1
+// PPC-AIX72:#define _AIX51 1
+// PPC-AIX72:#define _AIX52 1
+// PPC-AIX72:#define _AIX53 1
+// PPC-AIX72:#define _AIX61 1
+// PPC-AIX72:#define _AIX71 1
+// PPC-AIX72:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX71 %s
+//
+// PPC-AIX71:#define _AIX32 1
+// PPC-AIX71:#define _AIX41 1
+// PPC-AIX71:#define _AIX43 1
+// PPC-AIX71:#define _AIX50 1
+// PPC-AIX71:#define _AIX51 1
+// PPC-AIX71:#define _AIX52 1
+// PPC-AIX71:#define _AIX53 1
+// PPC-AIX71:#define _AIX61 1
+// PPC-AIX71:#define _AIX71 1
+// PPC-AIX71-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix6.1.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX61 %s
+//
+// PPC-AIX61:#define _AIX32 1
+// PPC-AIX61:#define _AIX41 1
+// PPC-AIX61:#define _AIX43 1
+// PPC-AIX61:#define _AIX50 1
+// PPC-AIX61:#define _AIX51 1
+// PPC-AIX61:#define _AIX52 1
+// PPC-AIX61:#define _AIX53 1
+// PPC-AIX61:#define _AIX61 1
+// PPC-AIX61-NOT:#define _AIX71 1
+// PPC-AIX61-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix5.3.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX53 %s
+// PPC-AIX53:#define _AIX32 1
+// PPC-AIX53:#define _AIX41 1
+// PPC-AIX53:#define _AIX43 1
+// PPC-AIX53:#define _AIX50 1
+// PPC-AIX53:#define _AIX51 1
+// PPC-AIX53:#define _AIX52 1
+// PPC-AIX53:#define _AIX53 1
+// PPC-AIX53-NOT:#define _AIX61 1
+// PPC-AIX53-NOT:#define _AIX71 1
+// PPC-AIX53-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix5.2.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX52 %s
+// PPC-AIX52:#define _AIX32 1
+// PPC-AIX52:#define _AIX41 1
+// PPC-AIX52:#define _AIX43 1
+// PPC-AIX52:#define _AIX50 1
+// PPC-AIX52:#define _AIX51 1
+// PPC-AIX52:#define _AIX52 1
+// PPC-AIX52-NOT:#define _AIX53 1
+// PPC-AIX52-NOT:#define _AIX61 1
+// PPC-AIX52-NOT:#define _AIX71 1
+// PPC-AIX52-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix5.1.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX51 %s
+// PPC-AIX51:#define _AIX32 1
+// PPC-AIX51:#define _AIX41 1
+// PPC-AIX51:#define _AIX43 1
+// PPC-AIX51:#define _AIX50 1
+// PPC-AIX51:#define _AIX51 1
+// PPC-AIX51-NOT:#define _AIX52 1
+// PPC-AIX51-NOT:#define _AIX53 1
+// PPC-AIX51-NOT:#define _AIX61 1
+// PPC-AIX51-NOT:#define _AIX71 1
+// PPC-AIX51-NOT:#define _AIX72 1
+//
+//RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix5.0.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX50 %s
+// PPC-AIX50:#define _AIX32 1
+// PPC-AIX50:#define _AIX41 1
+// PPC-AIX50:#define _AIX43 1
+// PPC-AIX50:#define _AIX50 1
+// PPC-AIX50-NOT:#define _AIX51 1
+// PPC-AIX50-NOT:#define _AIX52 1
+// PPC-AIX50-NOT:#define _AIX53 1
+// PPC-AIX50-NOT:#define _AIX61 1
+// PPC-AIX50-NOT:#define _AIX71 1
+// PPC-AIX50-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix4.3.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX43 %s
+// PPC-AIX43:#define _AIX32 1
+// PPC-AIX43:#define _AIX41 1
+// PPC-AIX43:#define _AIX43 1
+// PPC-AIX43-NOT:#define _AIX50 1
+// PPC-AIX43-NOT:#define _AIX51 1
+// PPC-AIX43-NOT:#define _AIX52 1
+// PPC-AIX43-NOT:#define _AIX53 1
+// PPC-AIX43-NOT:#define _AIX61 1
+// PPC-AIX43-NOT:#define _AIX71 1
+// PPC-AIX43-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix4.1.0.0 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX41 %s
+// PPC-AIX41:#define _AIX32 1
+// PPC-AIX41:#define _AIX41 1
+// PPC-AIX41-NOT:#define _AIX43 1
+// PPC-AIX41-NOT:#define _AIX50 1
+// PPC-AIX41-NOT:#define _AIX51 1
+// PPC-AIX41-NOT:#define _AIX52 1
+// PPC-AIX41-NOT:#define _AIX53 1
+// PPC-AIX41-NOT:#define _AIX61 1
+// PPC-AIX41-NOT:#define _AIX71 1
+// PPC-AIX41-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=po

r360900 - Add AIX Version Macros

[Xing Xue via cfe-commits]

Author: xingxue
Date: Thu May 16 07:22:37 2019
New Revision: 360900

URL: http://llvm.org/viewvc/llvm-project?rev=360900&view=rev
Log:
Add AIX Version Macros

Summary:
- This patch checks the AIX version and defines the appropriate macros.
- Follow up to a comment on D59048.

Author: andusy

Reviewers: hubert.reinterpretcast, jasonliu, sfertile, xingxue

Reviewed By: sfertile

Subscribers: jsji, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Basic/Targets/OSTargets.h
cfe/trunk/test/Preprocessor/init.c

Modified: cfe/trunk/lib/Basic/Targets/OSTargets.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/OSTargets.h?rev=360900&r1=360899&r2=360900&view=diff
==
--- cfe/trunk/lib/Basic/Targets/OSTargets.h (original)
+++ cfe/trunk/lib/Basic/Targets/OSTargets.h Thu May 16 07:22:37 2019
@@ -654,9 +654,25 @@ protected:
 Builder.defineMacro("_IBMR2");
 Builder.defineMacro("_POWER");
 
-// FIXME: Define AIX OS-Version Macros.
 Builder.defineMacro("_AIX");
 
+unsigned Major, Minor, Micro;
+Triple.getOSVersion(Major, Minor, Micro);
+
+// Define AIX OS-Version Macros.
+// Includes logic for legacy versions of AIX; no specific intent to 
support.
+std::pair OsVersion = {Major, Minor};
+if (OsVersion >= std::make_pair(3, 2)) Builder.defineMacro("_AIX32");
+if (OsVersion >= std::make_pair(4, 1)) Builder.defineMacro("_AIX41");
+if (OsVersion >= std::make_pair(4, 3)) Builder.defineMacro("_AIX43");
+if (OsVersion >= std::make_pair(5, 0)) Builder.defineMacro("_AIX50");
+if (OsVersion >= std::make_pair(5, 1)) Builder.defineMacro("_AIX51");
+if (OsVersion >= std::make_pair(5, 2)) Builder.defineMacro("_AIX52");
+if (OsVersion >= std::make_pair(5, 3)) Builder.defineMacro("_AIX53");
+if (OsVersion >= std::make_pair(6, 1)) Builder.defineMacro("_AIX61");
+if (OsVersion >= std::make_pair(7, 1)) Builder.defineMacro("_AIX71");
+if (OsVersion >= std::make_pair(7, 2)) Builder.defineMacro("_AIX72");
+
 // FIXME: Do not define _LONG_LONG when -fno-long-long is specified.
 Builder.defineMacro("_LONG_LONG");
 

Modified: cfe/trunk/test/Preprocessor/init.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/init.c?rev=360900&r1=360899&r2=360900&view=diff
==
--- cfe/trunk/test/Preprocessor/init.c (original)
+++ cfe/trunk/test/Preprocessor/init.c Thu May 16 07:22:37 2019
@@ -7243,6 +7243,129 @@
 // PPC-AIX:#define __powerpc__ 1
 // PPC-AIX:#define __ppc__ 1
 //
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.2.0.0 < 
/dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX72 %s
+//
+// PPC-AIX72:#define _AIX32 1
+// PPC-AIX72:#define _AIX41 1
+// PPC-AIX72:#define _AIX43 1
+// PPC-AIX72:#define _AIX50 1
+// PPC-AIX72:#define _AIX51 1
+// PPC-AIX72:#define _AIX52 1
+// PPC-AIX72:#define _AIX53 1
+// PPC-AIX72:#define _AIX61 1
+// PPC-AIX72:#define _AIX71 1
+// PPC-AIX72:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 < 
/dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX71 %s
+//
+// PPC-AIX71:#define _AIX32 1
+// PPC-AIX71:#define _AIX41 1
+// PPC-AIX71:#define _AIX43 1
+// PPC-AIX71:#define _AIX50 1
+// PPC-AIX71:#define _AIX51 1
+// PPC-AIX71:#define _AIX52 1
+// PPC-AIX71:#define _AIX53 1
+// PPC-AIX71:#define _AIX61 1
+// PPC-AIX71:#define _AIX71 1
+// PPC-AIX71-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix6.1.0.0 < 
/dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX61 %s
+//
+// PPC-AIX61:#define _AIX32 1
+// PPC-AIX61:#define _AIX41 1
+// PPC-AIX61:#define _AIX43 1
+// PPC-AIX61:#define _AIX50 1
+// PPC-AIX61:#define _AIX51 1
+// PPC-AIX61:#define _AIX52 1
+// PPC-AIX61:#define _AIX53 1
+// PPC-AIX61:#define _AIX61 1
+// PPC-AIX61-NOT:#define _AIX71 1
+// PPC-AIX61-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix5.3.0.0 < 
/dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX53 %s
+// PPC-AIX53:#define _AIX32 1
+// PPC-AIX53:#define _AIX41 1
+// PPC-AIX53:#define _AIX43 1
+// PPC-AIX53:#define _AIX50 1
+// PPC-AIX53:#define _AIX51 1
+// PPC-AIX53:#define _AIX52 1
+// PPC-AIX53:#define _AIX53 1
+// PPC-AIX53-NOT:#define _AIX61 1
+// PPC-AIX53-NOT:#define _AIX71 1
+// PPC-AIX53-NOT:#define _AIX72 1
+//
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix5.2.0.0 < 
/dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX52 %s
+// PPC-AIX52:#define _AIX32 1
+// PPC-AIX52:#define _AIX41 1
+// PPC-AIX52:#define _AIX43 1
+// PPC-AIX52:#define _AIX50 1
+// PPC-AIX52:#define _AIX51 1
+// PPC-AIX52:#define _AIX52 1
+// PPC-AIX52-NOT:#define _AIX53 1
+// PPC-AIX52-NOT:#define _AIX61 1
+// PPC-AIX52-NOT:#define _AIX71

[PATCH] D62010: [CodeComplete] Complete enumerators when preferred type is an enum

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: kadircet.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62010

Files:
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/enum-preferred-type.cpp

Index: clang/test/CodeCompletion/enum-preferred-type.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/enum-preferred-type.cpp
@@ -0,0 +1,21 @@
+namespace N {
+  enum Color {
+Red,
+Blue,
+Orange,
+  };
+}
+
+void test(N::Color color) {
+  color = N::Color::Red;
+  test(N::Color::Red);
+  if (color == N::Color::Red) {}
+
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:11 %s -o - | FileCheck %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:11:8 %s -o - | FileCheck %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:16 %s -o - | FileCheck %s
+  // CHECK: Blue : [#N::Color#]N::Blue
+  // CHECK: color : [#N::Color#]color
+  // CHECK: Orange : [#N::Color#]N::Orange
+  // CHECK: Red : [#N::Color#]N::Red
+}
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -4078,6 +4078,36 @@
   SmallVector IgnoreDecls;
 };
 
+namespace {
+/// Information that allows to avoid completing redundant enumerators.
+struct CoveredEnumerators {
+  llvm::SmallPtrSet Seen;
+  NestedNameSpecifier *SuggestedQualifier = nullptr;
+};
+} // namespace
+
+static void AddEnumerators(ResultBuilder &Results, ASTContext &Context,
+   EnumDecl *Enum, DeclContext *CurContext,
+   const CoveredEnumerators &Enumerators) {
+  NestedNameSpecifier *Qualifier = Enumerators.SuggestedQualifier;
+  if (Context.getLangOpts().CPlusPlus && !Qualifier && Enumerators.Seen.empty()) {
+// If there are no prior enumerators in C++, check whether we have to
+// qualify the names of the enumerators that we suggest, because they
+// may not be visible in this scope.
+Qualifier = getRequiredQualification(Context, CurContext, Enum);
+  }
+
+  Results.EnterNewScope();
+  for (auto *E : Enum->enumerators()) {
+if (Enumerators.Seen.count(E))
+  continue;
+
+CodeCompletionResult R(E, CCP_EnumInCase, Qualifier);
+Results.AddResult(R, CurContext, nullptr, false);
+  }
+  Results.ExitScope();
+}
+
 /// Perform code-completion in an expression context when we know what
 /// type we're looking for.
 void Sema::CodeCompleteExpression(Scope *S,
@@ -4118,10 +4148,19 @@
   Results.ExitScope();
 
   bool PreferredTypeIsPointer = false;
-  if (!Data.PreferredType.isNull())
+  if (!Data.PreferredType.isNull()) {
 PreferredTypeIsPointer = Data.PreferredType->isAnyPointerType() ||
  Data.PreferredType->isMemberPointerType() ||
  Data.PreferredType->isBlockPointerType();
+if (Data.PreferredType->isEnumeralType()) {
+  EnumDecl *Enum = Data.PreferredType->castAs()->getDecl();
+  if (auto *Def = Enum->getDefinition())
+Enum = Def;
+  // FIXME: collect covered enumerators in cases like:
+  //if (x == my_enum::one) { ... } else if (x == ^) {}
+  AddEnumerators(Results, Context, Enum, CurContext, CoveredEnumerators());
+}
+  }
 
   if (S->getFnParent() && !Data.ObjCCollection &&
   !Data.IntegralConstantExpression)
@@ -4719,8 +4758,7 @@
   // FIXME: Ideally, we would also be able to look *past* the code-completion
   // token, in case we are code-completing in the middle of the switch and not
   // at the end. However, we aren't able to do so at the moment.
-  llvm::SmallPtrSet EnumeratorsSeen;
-  NestedNameSpecifier *Qualifier = nullptr;
+  CoveredEnumerators Enumerators;
   for (SwitchCase *SC = Switch->getSwitchCaseList(); SC;
SC = SC->getNextSwitchCase()) {
 CaseStmt *Case = dyn_cast(SC);
@@ -4737,7 +4775,7 @@
 // values of each enumerator. However, value-based approach would not
 // work as well with C++ templates where enumerators declared within a
 // template are type- and value-dependent.
-EnumeratorsSeen.insert(Enumerator);
+Enumerators.Seen.insert(Enumerator);
 
 // If this is a qualified-id, keep track of the nested-name-specifier
 // so that we can reproduce it as part of code completion, e.g.,
@@ -4750,30 +4788,15 @@
 // At the XXX, our completions are TagDecl::TK_union,
 // TagDecl::TK_struct, and TagDecl::TK_class, rather than TK_union,
 // TK_struct, and TK_class.
-Qualifier = DRE->getQualifier();
+Enumerators.SuggestedQualifier = DRE->getQualifier();
   }
   }
 
-  if (getLangOpts().CPlusPlus && !Qualifier && EnumeratorsSeen.empty()) {
-// If there are no prior enumerators in C++, check whether we have to
-// qualify the names of the enumerators

[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for this -- it looks like really interesting functionality! I've mostly 
found nits thus far, but did have a question about clang-query support for it.




Comment at: include/clang/AST/ASTContext.h:562-563
 public:
+  ast_type_traits::TraversalKind GetTraversalKind() const { return Traversal; }
+  void SetTraversalKind(ast_type_traits::TraversalKind TK) { Traversal = TK; }
+

This doesn't match our coding guidelines. Should be `getTraversalKind()`, etc. 
Same below.



Comment at: include/clang/AST/ASTContext.h:565-568
+  const Expr *TraverseIgnored(const Expr *E);
+  Expr *TraverseIgnored(Expr *E);
+  ast_type_traits::DynTypedNode
+  TraverseIgnored(const ast_type_traits::DynTypedNode &N);

Is there a reason we don't want these functions to be marked `const`?



Comment at: include/clang/AST/ASTNodeTraverser.h:80
 
+  void SetTraversalKind(ast_type_traits::TraversalKind TK) { Traversal = TK; }
+

`setTraversalKind()`



Comment at: include/clang/AST/ASTNodeTraverser.h:105
 
-  void Visit(const Stmt *S, StringRef Label = {}) {
+  void Visit(const Stmt *S_, StringRef Label = {}) {
 getNodeDelegate().AddChild(Label, [=] {

Let's not make underscores important like this -- how about `Node` or something 
generic instead?



Comment at: include/clang/AST/ASTNodeTraverser.h:113-114
+  break;
+case ast_type_traits::TK_IgnoreImplicitCastsAndParentheses:
+  S = E->IgnoreParenImpCasts();
+  break;

What an unfortunate name for this. `IgnoreParenImpCasts()` ignores parens, 
implicit casts, full expressions, temporary materialization, and non-type 
template substitutions, and those extra nodes have surprised people in the past.

Not much to be done about it here, just loudly lamenting the existing name of 
the trait.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:701
 
+/// Causes all nested matchers to be matched with the specified traversal kind
+///

Missing a full stop at the end of the comment.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:716
+/// \endcode
+/// matches the return statement with "ret" bound to "a".
+template 

Copy pasta?



Comment at: include/clang/ASTMatchers/ASTMatchers.h:718
+template 
+internal::Matcher traverse(ast_type_traits::TraversalKind TK,
+  const internal::Matcher &InnerMatcher) {

Is this an API we should be exposing to clang-query as well? Will those users 
be able to use a string literal for the traversal kind, like they already do 
for attribute kinds (etc)?



Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:286
+
+  virtual llvm::Optional TraversalKind() const 
{
+return {};

`traversalKind()`



Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:287
+  virtual llvm::Optional TraversalKind() const 
{
+return {};
+  }

`return llvm::None;`



Comment at: lib/AST/ASTContext.cpp:120
+ASTContext::TraverseIgnored(const ast_type_traits::DynTypedNode &N) {
+  if (auto E = N.get()) {
+return ast_type_traits::DynTypedNode::create(*TraverseIgnored(E));

`auto *`



Comment at: lib/AST/ASTContext.cpp:10358
   bool TraverseStmt(Stmt *StmtNode) {
+auto FilteredNode = StmtNode;
+if (auto *ExprNode = dyn_cast_or_null(FilteredNode))

`Stmt *`



Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:148
 Stmt *StmtToTraverse = StmtNode;
+if (Expr *ExprNode = dyn_cast_or_null(StmtNode))
+  StmtToTraverse = Finder->getASTContext().TraverseIgnored(ExprNode);

`auto *`



Comment at: lib/ASTMatchers/ASTMatchersInternal.cpp:214-215
   BoundNodesTreeBuilder *Builder) const {
-  if (RestrictKind.isBaseOf(DynNode.getNodeKind()) &&
-  Implementation->dynMatches(DynNode, Finder, Builder)) {
+  auto PreviousTraversalKind = Finder->getASTContext().GetTraversalKind();
+  auto OptTK = Implementation->TraversalKind();
+  if (OptTK)

Please do not use `auto` here.



Comment at: lib/ASTMatchers/ASTMatchersInternal.cpp:218-219
+Finder->getASTContext().SetTraversalKind(*OptTK);
+  auto N = Finder->getASTContext().TraverseIgnored(DynNode);
+  auto NodeKind = N.getNodeKind();
+

Or here...



Comment at: lib/ASTMatchers/ASTMatchersInternal.cpp:237-238
 BoundNodesTreeBuilder *Builder) const {
-  assert(RestrictKind.isBaseOf(DynNode.getNodeKind()));
-  if (Implementation->dynMatches(DynNode, Finder, Builder)) {
+  auto N = Finder->getASTContext().TraverseIgnored(DynNode);
+  auto NodeKind = N.getNodeKind();
+

[PATCH] D61836: Move TraversalKind enum to ast_type_traits

[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 some commenting requests.




Comment at: include/clang/AST/ASTTypeTraits.h:44
+enum TraversalKind {
+  /// Will traverse any child nodes.
+  TK_AsIs,

s/any/all ?



Comment at: include/clang/AST/ASTTypeTraits.h:47
+
+  /// Will not traverse implicit casts and parentheses.
+  TK_IgnoreImplicitCastsAndParentheses

Can you update the comment to more clearly state what is skipped, because it's 
more than just implicit casts and parentheses?

If we can find a better name for the traversal kind, then great, but I can't 
think of one. `IgnoreRandomImplicitJunk` doesn't seem like an improvement, 
despite it being more accurate. :-P


Repository:
  rC Clang

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

https://reviews.llvm.org/D61836



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

[PATCH] D62009: [clang] perform semantic checking in constant context

[Tyker via Phabricator via cfe-commits]

Tyker updated this revision to Diff 199830.

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

https://reviews.llvm.org/D62009

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/builtin-is-constant-evaluated.cpp

Index: clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
===
--- clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
+++ clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
@@ -119,3 +119,11 @@
 };
 TestConditionalExplicit e = 42;
 #endif
+
+constexpr int i = (long long)__builtin_is_constant_evaluated() * (1ll << 33);
+// expected-warning@-1 {{implicit conversion}}
+
+void f() {
+  if constexpr ((long long)__builtin_is_constant_evaluated() * (1ll << 33))
+ ;// expected-error@-1 {{cannot be narrowed to type 'bool'}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5408,6 +5408,7 @@
   if (checkPlaceholderForOverload(S, From))
 return ExprError();
 
+  From->setIsConstantContext();
   // C++1z [expr.const]p3:
   //  A converted constant expression of type T is an expression,
   //  implicitly converted to type T, where the converted
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -11100,6 +11100,10 @@
   return;
   }
 
+  Init->setIsConstantContext(
+  VDecl->isConstexpr() ||
+  (VDecl->getType().isConstQualified() && !Init->isValueDependent()));
+
   // dllimport cannot be used on variable definitions.
   if (VDecl->hasAttr() && !VDecl->isStaticDataMember()) {
 Diag(VDecl->getLocation(), diag::err_attribute_dllimport_data_definition);
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11439,7 +11439,7 @@
 bool Expr::EvaluateAsRValue(EvalResult &Result, const ASTContext &Ctx,
 bool InConstantContext) const {
   EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);
-  Info.InConstantContext = InConstantContext;
+  Info.InConstantContext = InConstantContext || IsConstantContext();
   return ::EvaluateAsRValue(this, Result, Ctx, Info);
 }
 
@@ -11453,12 +11453,14 @@
 bool Expr::EvaluateAsInt(EvalResult &Result, const ASTContext &Ctx,
  SideEffectsKind AllowSideEffects) const {
   EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);
+  Info.InConstantContext = IsConstantContext();
   return ::EvaluateAsInt(this, Result, Ctx, AllowSideEffects, Info);
 }
 
 bool Expr::EvaluateAsFixedPoint(EvalResult &Result, const ASTContext &Ctx,
 SideEffectsKind AllowSideEffects) const {
   EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);
+  Info.InConstantContext = IsConstantContext();
   return ::EvaluateAsFixedPoint(this, Result, Ctx, AllowSideEffects, Info);
 }
 
@@ -11478,7 +11480,7 @@
 
 bool Expr::EvaluateAsLValue(EvalResult &Result, const ASTContext &Ctx) const {
   EvalInfo Info(Ctx, Result, EvalInfo::EM_ConstantFold);
-
+  Info.InConstantContext = IsConstantContext();
   LValue LV;
   if (!EvaluateLValue(this, LV, Info) || Result.HasSideEffects ||
   !CheckLValueConstantExpression(Info, getExprLoc(),
@@ -11588,6 +11590,7 @@
   EvalResult EVResult;
   if (!FastEvaluateAsRValue(this, EVResult, Ctx, IsConst)) {
 EvalInfo Info(Ctx, EVResult, EvalInfo::EM_EvaluateForOverflow);
+Info.InConstantContext = IsConstantContext();
 (void)::EvaluateAsRValue(Info, this, EVResult.Val);
   }
 }
@@ -12115,6 +12118,7 @@
   SmallVector Diags;
   Status.Diag = &Diags;
   EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantExpression);
+  Info.InConstantContext = IsConstantContext();
 
   APValue Scratch;
   bool IsConstExpr = ::EvaluateAsRValue(Info, this, Result ? *Result : Scratch);
Index: clang/include/clang/AST/Stmt.h
===
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -318,8 +318,9 @@
 unsigned ValueDependent : 1;
 unsigned InstantiationDependent : 1;
 unsigned ContainsUnexpandedParameterPack : 1;
+unsigned IsConstantContext : 1;
   };
-  enum { NumExprBits = NumStmtBits + 9 };
+  enum { NumExprBits = NumStmtBits + 10 };
 
   class PredefinedExprBitfields {
 friend class ASTStmtReader;
Index: clang/include/clang/AST/Expr.h
===
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -223,6 +223,18 @@
 ExprBits.ContainsUnexpandedParameterPack = PP;
   }
 
+  /// Wether this expressi

[PATCH] D61950: [PowerPC64] adds ABI parsing when specified on target triple

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

Thanks for the explanation!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61950



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

[PATCH] D61989: [clang-tidy] enable modernize-concat-nested-namespaces on header files

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Will be good idea to mention new option in the Release Notes.




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst:56
+   A comma-separated list of filename extensions of header files (the filename
+   extensions should not contain "." prefix). Default is "h,hh,hpp,hxx".
+   For header files without an extension, use an empty string (if there are no

Please use single back-ticks for highlighting option values. Same in other 
places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61989



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

[PATCH] D62010: [CodeComplete] Complete enumerators when preferred type is an enum

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

LGTM, thanks!




Comment at: clang/test/CodeCompletion/enum-preferred-type.cpp:13
+  if (color == N::Color::Red) {}
+
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:11 %s -o - | 
FileCheck %s

Could you also add an `else if` case, with a similar fixme as in the code?
So that when someone fixes it, we can be sure that fix is working.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62010



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

r360907 - ftime-trace as a CoreOption

[Alexandre Ganea via cfe-commits]

Author: aganea
Date: Thu May 16 08:14:01 2019
New Revision: 360907

URL: http://llvm.org/viewvc/llvm-project?rev=360907&view=rev
Log:
ftime-trace as a CoreOption

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

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

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=360907&r1=360906&r2=360907&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu May 16 08:14:01 2019
@@ -1747,7 +1747,7 @@ def Wframe_larger_than_EQ : Joined<["-"]
 def : Flag<["-"], "fterminated-vtables">, Alias;
 def fthreadsafe_statics : Flag<["-"], "fthreadsafe-statics">, Group;
 def ftime_report : Flag<["-"], "ftime-report">, Group, 
Flags<[CC1Option]>;
-def ftime_trace : Flag<["-"], "ftime-trace">, Group, 
Flags<[CC1Option]>;
+def ftime_trace : Flag<["-"], "ftime-trace">, Group, 
Flags<[CC1Option, CoreOption]>;
 def ftlsmodel_EQ : Joined<["-"], "ftls-model=">, Group, 
Flags<[CC1Option]>;
 def ftrapv : Flag<["-"], "ftrapv">, Group, Flags<[CC1Option]>,
   HelpText<"Trap on integer overflow">;

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=360907&r1=360906&r2=360907&view=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Thu May 16 08:14:01 2019
@@ -638,6 +638,7 @@
 // RUN: -fno-profile-instr-use \
 // RUN: -fcs-profile-generate \
 // RUN: -fcs-profile-generate=dir \
+// RUN: -ftime-trace \
 // RUN: --version \
 // RUN: -Werror /Zs -- %s 2>&1
 


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

[PATCH] D61945: ftime-trace as a CoreOption

[Alexandre Ganea via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL360907: ftime-trace as a CoreOption (authored by aganea, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61945?vs=199604&id=199836#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61945

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


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1747,7 +1747,7 @@
 def : Flag<["-"], "fterminated-vtables">, Alias;
 def fthreadsafe_statics : Flag<["-"], "fthreadsafe-statics">, Group;
 def ftime_report : Flag<["-"], "ftime-report">, Group, 
Flags<[CC1Option]>;
-def ftime_trace : Flag<["-"], "ftime-trace">, Group, 
Flags<[CC1Option]>;
+def ftime_trace : Flag<["-"], "ftime-trace">, Group, 
Flags<[CC1Option, CoreOption]>;
 def ftlsmodel_EQ : Joined<["-"], "ftls-model=">, Group, 
Flags<[CC1Option]>;
 def ftrapv : Flag<["-"], "ftrapv">, Group, Flags<[CC1Option]>,
   HelpText<"Trap on integer overflow">;
Index: cfe/trunk/test/Driver/cl-options.c
===
--- cfe/trunk/test/Driver/cl-options.c
+++ cfe/trunk/test/Driver/cl-options.c
@@ -638,6 +638,7 @@
 // RUN: -fno-profile-instr-use \
 // RUN: -fcs-profile-generate \
 // RUN: -fcs-profile-generate=dir \
+// RUN: -ftime-trace \
 // RUN: --version \
 // RUN: -Werror /Zs -- %s 2>&1
 


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1747,7 +1747,7 @@
 def : Flag<["-"], "fterminated-vtables">, Alias;
 def fthreadsafe_statics : Flag<["-"], "fthreadsafe-statics">, Group;
 def ftime_report : Flag<["-"], "ftime-report">, Group, Flags<[CC1Option]>;
-def ftime_trace : Flag<["-"], "ftime-trace">, Group, Flags<[CC1Option]>;
+def ftime_trace : Flag<["-"], "ftime-trace">, Group, Flags<[CC1Option, CoreOption]>;
 def ftlsmodel_EQ : Joined<["-"], "ftls-model=">, Group, Flags<[CC1Option]>;
 def ftrapv : Flag<["-"], "ftrapv">, Group, Flags<[CC1Option]>,
   HelpText<"Trap on integer overflow">;
Index: cfe/trunk/test/Driver/cl-options.c
===
--- cfe/trunk/test/Driver/cl-options.c
+++ cfe/trunk/test/Driver/cl-options.c
@@ -638,6 +638,7 @@
 // RUN: -fno-profile-instr-use \
 // RUN: -fcs-profile-generate \
 // RUN: -fcs-profile-generate=dir \
+// RUN: -ftime-trace \
 // RUN: --version \
 // RUN: -Werror /Zs -- %s 2>&1
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62010: [CodeComplete] Complete enumerators when preferred type is an enum

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 199835.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- add a test case with a fixme


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62010

Files:
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/enum-preferred-type.cpp

Index: clang/test/CodeCompletion/enum-preferred-type.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/enum-preferred-type.cpp
@@ -0,0 +1,24 @@
+namespace N {
+  enum Color {
+Red,
+Blue,
+Orange,
+  };
+}
+
+void test(N::Color color) {
+  color = N::Color::Red;
+  test(N::Color::Red);
+  if (color == N::Color::Red) {}
+  // FIXME: ideally, we should not show 'Red' on the next line.
+  else if (color == N::Color::Blue) {}
+
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:11 %s -o - | FileCheck %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:11:8 %s -o - | FileCheck %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:16 %s -o - | FileCheck %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:21 %s -o - | FileCheck %s
+  // CHECK: Blue : [#N::Color#]N::Blue
+  // CHECK: color : [#N::Color#]color
+  // CHECK: Orange : [#N::Color#]N::Orange
+  // CHECK: Red : [#N::Color#]N::Red
+}
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -4078,6 +4078,36 @@
   SmallVector IgnoreDecls;
 };
 
+namespace {
+/// Information that allows to avoid completing redundant enumerators.
+struct CoveredEnumerators {
+  llvm::SmallPtrSet Seen;
+  NestedNameSpecifier *SuggestedQualifier = nullptr;
+};
+} // namespace
+
+static void AddEnumerators(ResultBuilder &Results, ASTContext &Context,
+   EnumDecl *Enum, DeclContext *CurContext,
+   const CoveredEnumerators &Enumerators) {
+  NestedNameSpecifier *Qualifier = Enumerators.SuggestedQualifier;
+  if (Context.getLangOpts().CPlusPlus && !Qualifier && Enumerators.Seen.empty()) {
+// If there are no prior enumerators in C++, check whether we have to
+// qualify the names of the enumerators that we suggest, because they
+// may not be visible in this scope.
+Qualifier = getRequiredQualification(Context, CurContext, Enum);
+  }
+
+  Results.EnterNewScope();
+  for (auto *E : Enum->enumerators()) {
+if (Enumerators.Seen.count(E))
+  continue;
+
+CodeCompletionResult R(E, CCP_EnumInCase, Qualifier);
+Results.AddResult(R, CurContext, nullptr, false);
+  }
+  Results.ExitScope();
+}
+
 /// Perform code-completion in an expression context when we know what
 /// type we're looking for.
 void Sema::CodeCompleteExpression(Scope *S,
@@ -4118,10 +4148,19 @@
   Results.ExitScope();
 
   bool PreferredTypeIsPointer = false;
-  if (!Data.PreferredType.isNull())
+  if (!Data.PreferredType.isNull()) {
 PreferredTypeIsPointer = Data.PreferredType->isAnyPointerType() ||
  Data.PreferredType->isMemberPointerType() ||
  Data.PreferredType->isBlockPointerType();
+if (Data.PreferredType->isEnumeralType()) {
+  EnumDecl *Enum = Data.PreferredType->castAs()->getDecl();
+  if (auto *Def = Enum->getDefinition())
+Enum = Def;
+  // FIXME: collect covered enumerators in cases like:
+  //if (x == my_enum::one) { ... } else if (x == ^) {}
+  AddEnumerators(Results, Context, Enum, CurContext, CoveredEnumerators());
+}
+  }
 
   if (S->getFnParent() && !Data.ObjCCollection &&
   !Data.IntegralConstantExpression)
@@ -4719,8 +4758,7 @@
   // FIXME: Ideally, we would also be able to look *past* the code-completion
   // token, in case we are code-completing in the middle of the switch and not
   // at the end. However, we aren't able to do so at the moment.
-  llvm::SmallPtrSet EnumeratorsSeen;
-  NestedNameSpecifier *Qualifier = nullptr;
+  CoveredEnumerators Enumerators;
   for (SwitchCase *SC = Switch->getSwitchCaseList(); SC;
SC = SC->getNextSwitchCase()) {
 CaseStmt *Case = dyn_cast(SC);
@@ -4737,7 +4775,7 @@
 // values of each enumerator. However, value-based approach would not
 // work as well with C++ templates where enumerators declared within a
 // template are type- and value-dependent.
-EnumeratorsSeen.insert(Enumerator);
+Enumerators.Seen.insert(Enumerator);
 
 // If this is a qualified-id, keep track of the nested-name-specifier
 // so that we can reproduce it as part of code completion, e.g.,
@@ -4750,30 +4788,15 @@
 // At the XXX, our completions are TagDecl::TK_union,
 // TagDecl::TK_struct, and TagDecl::TK_class, rather than TK_union,
 // TK_struct, and TK_class.
- 

[PATCH] D62010: [CodeComplete] Complete enumerators when preferred type is an enum

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang/test/CodeCompletion/enum-preferred-type.cpp:19
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:16 %s -o - | 
FileCheck %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:21 %s -o - | 
FileCheck %s
+  // CHECK: Blue : [#N::Color#]N::Blue

I suppose line number needs to be 14?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62010



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

[PATCH] D62010: [CodeComplete] Complete enumerators when preferred type is an enum

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks for a quick review!




Comment at: clang/test/CodeCompletion/enum-preferred-type.cpp:19
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:16 %s -o - | 
FileCheck %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:21 %s -o - | 
FileCheck %s
+  // CHECK: Blue : [#N::Color#]N::Blue

kadircet wrote:
> I suppose line number needs to be 14?
Right. Otherwise tests fail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62010



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

[PATCH] D62010: [CodeComplete] Complete enumerators when preferred type is an enum

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 199839.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.

- Fix a line number


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62010

Files:
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/enum-preferred-type.cpp

Index: clang/test/CodeCompletion/enum-preferred-type.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/enum-preferred-type.cpp
@@ -0,0 +1,24 @@
+namespace N {
+  enum Color {
+Red,
+Blue,
+Orange,
+  };
+}
+
+void test(N::Color color) {
+  color = N::Color::Red;
+  test(N::Color::Red);
+  if (color == N::Color::Red) {}
+  // FIXME: ideally, we should not show 'Red' on the next line.
+  else if (color == N::Color::Blue) {}
+
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:11 %s -o - | FileCheck %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:11:8 %s -o - | FileCheck %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:12:16 %s -o - | FileCheck %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:21 %s -o - | FileCheck %s
+  // CHECK: Blue : [#N::Color#]N::Blue
+  // CHECK: color : [#N::Color#]color
+  // CHECK: Orange : [#N::Color#]N::Orange
+  // CHECK: Red : [#N::Color#]N::Red
+}
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -4078,6 +4078,36 @@
   SmallVector IgnoreDecls;
 };
 
+namespace {
+/// Information that allows to avoid completing redundant enumerators.
+struct CoveredEnumerators {
+  llvm::SmallPtrSet Seen;
+  NestedNameSpecifier *SuggestedQualifier = nullptr;
+};
+} // namespace
+
+static void AddEnumerators(ResultBuilder &Results, ASTContext &Context,
+   EnumDecl *Enum, DeclContext *CurContext,
+   const CoveredEnumerators &Enumerators) {
+  NestedNameSpecifier *Qualifier = Enumerators.SuggestedQualifier;
+  if (Context.getLangOpts().CPlusPlus && !Qualifier && Enumerators.Seen.empty()) {
+// If there are no prior enumerators in C++, check whether we have to
+// qualify the names of the enumerators that we suggest, because they
+// may not be visible in this scope.
+Qualifier = getRequiredQualification(Context, CurContext, Enum);
+  }
+
+  Results.EnterNewScope();
+  for (auto *E : Enum->enumerators()) {
+if (Enumerators.Seen.count(E))
+  continue;
+
+CodeCompletionResult R(E, CCP_EnumInCase, Qualifier);
+Results.AddResult(R, CurContext, nullptr, false);
+  }
+  Results.ExitScope();
+}
+
 /// Perform code-completion in an expression context when we know what
 /// type we're looking for.
 void Sema::CodeCompleteExpression(Scope *S,
@@ -4118,10 +4148,19 @@
   Results.ExitScope();
 
   bool PreferredTypeIsPointer = false;
-  if (!Data.PreferredType.isNull())
+  if (!Data.PreferredType.isNull()) {
 PreferredTypeIsPointer = Data.PreferredType->isAnyPointerType() ||
  Data.PreferredType->isMemberPointerType() ||
  Data.PreferredType->isBlockPointerType();
+if (Data.PreferredType->isEnumeralType()) {
+  EnumDecl *Enum = Data.PreferredType->castAs()->getDecl();
+  if (auto *Def = Enum->getDefinition())
+Enum = Def;
+  // FIXME: collect covered enumerators in cases like:
+  //if (x == my_enum::one) { ... } else if (x == ^) {}
+  AddEnumerators(Results, Context, Enum, CurContext, CoveredEnumerators());
+}
+  }
 
   if (S->getFnParent() && !Data.ObjCCollection &&
   !Data.IntegralConstantExpression)
@@ -4719,8 +4758,7 @@
   // FIXME: Ideally, we would also be able to look *past* the code-completion
   // token, in case we are code-completing in the middle of the switch and not
   // at the end. However, we aren't able to do so at the moment.
-  llvm::SmallPtrSet EnumeratorsSeen;
-  NestedNameSpecifier *Qualifier = nullptr;
+  CoveredEnumerators Enumerators;
   for (SwitchCase *SC = Switch->getSwitchCaseList(); SC;
SC = SC->getNextSwitchCase()) {
 CaseStmt *Case = dyn_cast(SC);
@@ -4737,7 +4775,7 @@
 // values of each enumerator. However, value-based approach would not
 // work as well with C++ templates where enumerators declared within a
 // template are type- and value-dependent.
-EnumeratorsSeen.insert(Enumerator);
+Enumerators.Seen.insert(Enumerator);
 
 // If this is a qualified-id, keep track of the nested-name-specifier
 // so that we can reproduce it as part of code completion, e.g.,
@@ -4750,30 +4788,15 @@
 // At the XXX, our completions are TagDecl::TK_union,
 // TagDecl::TK_struct, and TagDecl::TK_class, rather than TK_union,
 // TK_struct, and TK_class.
-Qual

r360910 - [analyzer] Add a test plugin for checker option handling

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Thu May 16 08:55:07 2019
New Revision: 360910

URL: http://llvm.org/viewvc/llvm-project?rev=360910&view=rev
Log:
[analyzer] Add a test plugin for checker option handling

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

Added:
cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/
cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt

cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp

cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandlingAnalyzerPlugin.exports
Modified:
cfe/trunk/test/Analysis/checker-plugins.c
cfe/trunk/test/Analysis/plugins/CMakeLists.txt

Modified: cfe/trunk/test/Analysis/checker-plugins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/checker-plugins.c?rev=360910&r1=360909&r2=360910&view=diff
==
--- cfe/trunk/test/Analysis/checker-plugins.c (original)
+++ cfe/trunk/test/Analysis/checker-plugins.c Thu May 16 08:55:07 2019
@@ -30,3 +30,18 @@ void caller() {
 
 // CHECK-IMPLICITLY-DISABLED-NOT: example.Dependency
 // CHECK-IMPLICITLY-DISABLED-NOT: example.DependendentChecker
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load %llvmshlibdir/CheckerOptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CHECKER-OPTION-OUTPUT
+
+// CHECK-CHECKER-OPTION-OUTPUT: Example option is set to false
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load %llvmshlibdir/CheckerOptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   -analyzer-config example.MyChecker:ExampleOption=true \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CHECKER-OPTION-OUTPUT-TRUE
+
+// CHECK-CHECKER-OPTION-OUTPUT-TRUE: Example option is set to true

Modified: cfe/trunk/test/Analysis/plugins/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/plugins/CMakeLists.txt?rev=360910&r1=360909&r2=360910&view=diff
==
--- cfe/trunk/test/Analysis/plugins/CMakeLists.txt (original)
+++ cfe/trunk/test/Analysis/plugins/CMakeLists.txt Thu May 16 08:55:07 2019
@@ -1,9 +1,11 @@
 add_subdirectory(SampleAnalyzer)
 add_subdirectory(CheckerDependencyHandling)
+add_subdirectory(CheckerOptionHandling)
 
 set(CLANG_ANALYZER_PLUGIN_DEPS
   SampleAnalyzerPlugin
   CheckerDependencyHandlingAnalyzerPlugin
+  CheckerOptionHandlingAnalyzerPlugin
   )
 
 add_custom_target(clang-analyzer-plugin

Added: cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt?rev=360910&view=auto
==
--- cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt (added)
+++ cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt Thu 
May 16 08:55:07 2019
@@ -0,0 +1,11 @@
+set(LLVM_EXPORTED_SYMBOL_FILE 
${CMAKE_CURRENT_SOURCE_DIR}/CheckerOptionHandlingAnalyzerPlugin.exports)
+add_llvm_library(CheckerOptionHandlingAnalyzerPlugin MODULE 
CheckerOptionHandling.cpp PLUGIN_TOOL clang)
+
+if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
+  target_link_libraries(CheckerOptionHandlingAnalyzerPlugin PRIVATE
+clangAnalysis
+clangAST
+clangStaticAnalyzerCore
+LLVMSupport
+)
+endif()

Added: 
cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp?rev=360910&view=auto
==
--- 
cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp 
(added)
+++ 
cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp 
Thu May 16 08:55:07 2019
@@ -0,0 +1,43 @@
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+struct MyChecker : public Checker {
+  void checkBeginFunction(CheckerContext &Ctx) const {}
+};
+
+void registerMyChecker(CheckerManager &Mgr) {
+  MyChecker *Checker = Mgr.registerChecker();
+  llvm::outs() << "Example option is set to "
+   << (Mgr.getAnalyzerOptions().getCheckerBooleanOption(
+   Checker, "ExampleOption", false)
+   ? "true"
+   : "false")
+   << '\n';
+}
+
+bool shouldRegisterMyChecker(const LangOptions &LO) { return true; }
+
+} // end anonymous namespace
+
+// Register plugin!
+extern "C" void clang_registerC

[PATCH] D61756: Add a __FILE_NAME__ macro.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D61756#1504109 , @kristina wrote:

> Reverted in rL360842  as Windows bots were 
> failing.
>
> I suspect the `MSVCCompat` case may need to be handled differently depending 
> on the host OS similar to `PPDirectives.cpp`:
>
> if (LangOpts.MSVCCompat) {
>   NormalizedPath = Filename.str();
>   #ifndef _WIN32
>   llvm::sys::path::native(NormalizedPath);
>   #endif
> }
>
>
> Reopening this for now, will try to get a local Windows builder set up and 
> work out what the problem is before relanding.


Can't you just always use `llvm::sys::path::filename(PLFileName)`?  It defaults 
to the native style.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61756



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

[PATCH] D59465: [analyzer] Add a test plugin for checker option handling

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

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL360910: [analyzer] Add a test plugin for checker option 
handling (authored by Szelethus, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59465?vs=197210&id=199840#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59465

Files:
  cfe/trunk/test/Analysis/checker-plugins.c
  cfe/trunk/test/Analysis/plugins/CMakeLists.txt
  cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
  
cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
  
cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandlingAnalyzerPlugin.exports

Index: cfe/trunk/test/Analysis/plugins/CMakeLists.txt
===
--- cfe/trunk/test/Analysis/plugins/CMakeLists.txt
+++ cfe/trunk/test/Analysis/plugins/CMakeLists.txt
@@ -1,9 +1,11 @@
 add_subdirectory(SampleAnalyzer)
 add_subdirectory(CheckerDependencyHandling)
+add_subdirectory(CheckerOptionHandling)
 
 set(CLANG_ANALYZER_PLUGIN_DEPS
   SampleAnalyzerPlugin
   CheckerDependencyHandlingAnalyzerPlugin
+  CheckerOptionHandlingAnalyzerPlugin
   )
 
 add_custom_target(clang-analyzer-plugin
Index: cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
===
--- cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
+++ cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
@@ -0,0 +1,11 @@
+set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/CheckerOptionHandlingAnalyzerPlugin.exports)
+add_llvm_library(CheckerOptionHandlingAnalyzerPlugin MODULE CheckerOptionHandling.cpp PLUGIN_TOOL clang)
+
+if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
+  target_link_libraries(CheckerOptionHandlingAnalyzerPlugin PRIVATE
+clangAnalysis
+clangAST
+clangStaticAnalyzerCore
+LLVMSupport
+)
+endif()
Index: cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandlingAnalyzerPlugin.exports
===
--- cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandlingAnalyzerPlugin.exports
+++ cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandlingAnalyzerPlugin.exports
@@ -0,0 +1,2 @@
+clang_registerCheckers
+clang_analyzerAPIVersionString
Index: cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
===
--- cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
+++ cfe/trunk/test/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
@@ -0,0 +1,43 @@
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+struct MyChecker : public Checker {
+  void checkBeginFunction(CheckerContext &Ctx) const {}
+};
+
+void registerMyChecker(CheckerManager &Mgr) {
+  MyChecker *Checker = Mgr.registerChecker();
+  llvm::outs() << "Example option is set to "
+   << (Mgr.getAnalyzerOptions().getCheckerBooleanOption(
+   Checker, "ExampleOption", false)
+   ? "true"
+   : "false")
+   << '\n';
+}
+
+bool shouldRegisterMyChecker(const LangOptions &LO) { return true; }
+
+} // end anonymous namespace
+
+// Register plugin!
+extern "C" void clang_registerCheckers(CheckerRegistry ®istry) {
+  registry.addChecker(registerMyChecker, shouldRegisterMyChecker,
+  "example.MyChecker", "Example Description",
+  "example.mychecker.documentation.nonexistent.html",
+  /*isHidden*/false);
+
+  registry.addCheckerOption(/*OptionType*/ "bool",
+/*CheckerFullName*/ "example.MyChecker",
+/*OptionName*/ "ExampleOption",
+/*DefaultValStr*/ "false",
+/*Description*/ "This is an example checker opt.");
+}
+
+extern "C" const char clang_analyzerAPIVersionString[] =
+CLANG_ANALYZER_API_VERSION_STRING;
Index: cfe/trunk/test/Analysis/checker-plugins.c
===
--- cfe/trunk/test/Analysis/checker-plugins.c
+++ cfe/trunk/test/Analysis/checker-plugins.c
@@ -30,3 +30,18 @@
 
 // CHECK-IMPLICITLY-DISABLED-NOT: example.Dependency
 // CHECK-IMPLICITLY-DISABLED-NOT: example.DependendentChecker
+
+// RUN: %clang_analyze_cc1 %s 

r360912 - [CodeComplete] Complete enumerators when preferred type is an enum

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu May 16 09:06:57 2019
New Revision: 360912

URL: http://llvm.org/viewvc/llvm-project?rev=360912&view=rev
Log:
[CodeComplete] Complete enumerators when preferred type is an enum

Reviewers: kadircet

Reviewed By: kadircet

Subscribers: cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/CodeCompletion/enum-preferred-type.cpp
Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=360912&r1=360911&r2=360912&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Thu May 16 09:06:57 2019
@@ -4078,6 +4078,36 @@ struct Sema::CodeCompleteExpressionData
   SmallVector IgnoreDecls;
 };
 
+namespace {
+/// Information that allows to avoid completing redundant enumerators.
+struct CoveredEnumerators {
+  llvm::SmallPtrSet Seen;
+  NestedNameSpecifier *SuggestedQualifier = nullptr;
+};
+} // namespace
+
+static void AddEnumerators(ResultBuilder &Results, ASTContext &Context,
+   EnumDecl *Enum, DeclContext *CurContext,
+   const CoveredEnumerators &Enumerators) {
+  NestedNameSpecifier *Qualifier = Enumerators.SuggestedQualifier;
+  if (Context.getLangOpts().CPlusPlus && !Qualifier && 
Enumerators.Seen.empty()) {
+// If there are no prior enumerators in C++, check whether we have to
+// qualify the names of the enumerators that we suggest, because they
+// may not be visible in this scope.
+Qualifier = getRequiredQualification(Context, CurContext, Enum);
+  }
+
+  Results.EnterNewScope();
+  for (auto *E : Enum->enumerators()) {
+if (Enumerators.Seen.count(E))
+  continue;
+
+CodeCompletionResult R(E, CCP_EnumInCase, Qualifier);
+Results.AddResult(R, CurContext, nullptr, false);
+  }
+  Results.ExitScope();
+}
+
 /// Perform code-completion in an expression context when we know what
 /// type we're looking for.
 void Sema::CodeCompleteExpression(Scope *S,
@@ -4118,10 +4148,19 @@ void Sema::CodeCompleteExpression(Scope
   Results.ExitScope();
 
   bool PreferredTypeIsPointer = false;
-  if (!Data.PreferredType.isNull())
+  if (!Data.PreferredType.isNull()) {
 PreferredTypeIsPointer = Data.PreferredType->isAnyPointerType() ||
  Data.PreferredType->isMemberPointerType() ||
  Data.PreferredType->isBlockPointerType();
+if (Data.PreferredType->isEnumeralType()) {
+  EnumDecl *Enum = Data.PreferredType->castAs()->getDecl();
+  if (auto *Def = Enum->getDefinition())
+Enum = Def;
+  // FIXME: collect covered enumerators in cases like:
+  //if (x == my_enum::one) { ... } else if (x == ^) {}
+  AddEnumerators(Results, Context, Enum, CurContext, CoveredEnumerators());
+}
+  }
 
   if (S->getFnParent() && !Data.ObjCCollection &&
   !Data.IntegralConstantExpression)
@@ -4719,8 +4758,7 @@ void Sema::CodeCompleteCase(Scope *S) {
   // FIXME: Ideally, we would also be able to look *past* the code-completion
   // token, in case we are code-completing in the middle of the switch and not
   // at the end. However, we aren't able to do so at the moment.
-  llvm::SmallPtrSet EnumeratorsSeen;
-  NestedNameSpecifier *Qualifier = nullptr;
+  CoveredEnumerators Enumerators;
   for (SwitchCase *SC = Switch->getSwitchCaseList(); SC;
SC = SC->getNextSwitchCase()) {
 CaseStmt *Case = dyn_cast(SC);
@@ -4737,7 +4775,7 @@ void Sema::CodeCompleteCase(Scope *S) {
 // values of each enumerator. However, value-based approach would not
 // work as well with C++ templates where enumerators declared within a
 // template are type- and value-dependent.
-EnumeratorsSeen.insert(Enumerator);
+Enumerators.Seen.insert(Enumerator);
 
 // If this is a qualified-id, keep track of the nested-name-specifier
 // so that we can reproduce it as part of code completion, e.g.,
@@ -4750,30 +4788,15 @@ void Sema::CodeCompleteCase(Scope *S) {
 // At the XXX, our completions are TagDecl::TK_union,
 // TagDecl::TK_struct, and TagDecl::TK_class, rather than TK_union,
 // TK_struct, and TK_class.
-Qualifier = DRE->getQualifier();
+Enumerators.SuggestedQualifier = DRE->getQualifier();
   }
   }
 
-  if (getLangOpts().CPlusPlus && !Qualifier && EnumeratorsSeen.empty()) {
-// If there are no prior enumerators in C++, check whether we have to
-// qualify the names of the enumerators that we suggest, because they
-// may not be visible in this scope.
-Qualifier = getRequiredQualification(Context, CurContext, Enum);
-  }
-
   // Add any enumerators that have not yet been mentioned.
   ResultBuilder Results(*this, CodeCompleter->g

[PATCH] D62010: [CodeComplete] Complete enumerators when preferred type is an enum

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL360912: [CodeComplete] Complete enumerators when preferred 
type is an enum (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62010?vs=199839&id=199842#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62010

Files:
  cfe/trunk/lib/Sema/SemaCodeComplete.cpp
  cfe/trunk/test/CodeCompletion/enum-preferred-type.cpp

Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -4078,6 +4078,36 @@
   SmallVector IgnoreDecls;
 };
 
+namespace {
+/// Information that allows to avoid completing redundant enumerators.
+struct CoveredEnumerators {
+  llvm::SmallPtrSet Seen;
+  NestedNameSpecifier *SuggestedQualifier = nullptr;
+};
+} // namespace
+
+static void AddEnumerators(ResultBuilder &Results, ASTContext &Context,
+   EnumDecl *Enum, DeclContext *CurContext,
+   const CoveredEnumerators &Enumerators) {
+  NestedNameSpecifier *Qualifier = Enumerators.SuggestedQualifier;
+  if (Context.getLangOpts().CPlusPlus && !Qualifier && Enumerators.Seen.empty()) {
+// If there are no prior enumerators in C++, check whether we have to
+// qualify the names of the enumerators that we suggest, because they
+// may not be visible in this scope.
+Qualifier = getRequiredQualification(Context, CurContext, Enum);
+  }
+
+  Results.EnterNewScope();
+  for (auto *E : Enum->enumerators()) {
+if (Enumerators.Seen.count(E))
+  continue;
+
+CodeCompletionResult R(E, CCP_EnumInCase, Qualifier);
+Results.AddResult(R, CurContext, nullptr, false);
+  }
+  Results.ExitScope();
+}
+
 /// Perform code-completion in an expression context when we know what
 /// type we're looking for.
 void Sema::CodeCompleteExpression(Scope *S,
@@ -4118,10 +4148,19 @@
   Results.ExitScope();
 
   bool PreferredTypeIsPointer = false;
-  if (!Data.PreferredType.isNull())
+  if (!Data.PreferredType.isNull()) {
 PreferredTypeIsPointer = Data.PreferredType->isAnyPointerType() ||
  Data.PreferredType->isMemberPointerType() ||
  Data.PreferredType->isBlockPointerType();
+if (Data.PreferredType->isEnumeralType()) {
+  EnumDecl *Enum = Data.PreferredType->castAs()->getDecl();
+  if (auto *Def = Enum->getDefinition())
+Enum = Def;
+  // FIXME: collect covered enumerators in cases like:
+  //if (x == my_enum::one) { ... } else if (x == ^) {}
+  AddEnumerators(Results, Context, Enum, CurContext, CoveredEnumerators());
+}
+  }
 
   if (S->getFnParent() && !Data.ObjCCollection &&
   !Data.IntegralConstantExpression)
@@ -4719,8 +4758,7 @@
   // FIXME: Ideally, we would also be able to look *past* the code-completion
   // token, in case we are code-completing in the middle of the switch and not
   // at the end. However, we aren't able to do so at the moment.
-  llvm::SmallPtrSet EnumeratorsSeen;
-  NestedNameSpecifier *Qualifier = nullptr;
+  CoveredEnumerators Enumerators;
   for (SwitchCase *SC = Switch->getSwitchCaseList(); SC;
SC = SC->getNextSwitchCase()) {
 CaseStmt *Case = dyn_cast(SC);
@@ -4737,7 +4775,7 @@
 // values of each enumerator. However, value-based approach would not
 // work as well with C++ templates where enumerators declared within a
 // template are type- and value-dependent.
-EnumeratorsSeen.insert(Enumerator);
+Enumerators.Seen.insert(Enumerator);
 
 // If this is a qualified-id, keep track of the nested-name-specifier
 // so that we can reproduce it as part of code completion, e.g.,
@@ -4750,30 +4788,15 @@
 // At the XXX, our completions are TagDecl::TK_union,
 // TagDecl::TK_struct, and TagDecl::TK_class, rather than TK_union,
 // TK_struct, and TK_class.
-Qualifier = DRE->getQualifier();
+Enumerators.SuggestedQualifier = DRE->getQualifier();
   }
   }
 
-  if (getLangOpts().CPlusPlus && !Qualifier && EnumeratorsSeen.empty()) {
-// If there are no prior enumerators in C++, check whether we have to
-// qualify the names of the enumerators that we suggest, because they
-// may not be visible in this scope.
-Qualifier = getRequiredQualification(Context, CurContext, Enum);
-  }
-
   // Add any enumerators that have not yet been mentioned.
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(),
 CodeCompletionContext::CCC_Expression);
-  Results.EnterNewScope();
-  for (auto *E : Enum->enumerators()) {
-if (EnumeratorsSeen.count

[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type name/access index

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song updated this revision to Diff 199843.
yonghong-song retitled this revision from "[BPF] Preserve original struct/union 
type name/access index and array subscripts" to "[BPF] Preserve debuginfo 
array/union/struct type name/access index".
yonghong-song edited the summary of this revision.
yonghong-song added reviewers: rsmith, lebedev.ri.
yonghong-song added a comment.

do not generate gep any more, only intrinsics


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/Targets/BPF.h
  lib/CodeGen/CGBuilder.h
  lib/CodeGen/CGExpr.cpp
  test/CodeGen/bpf-offsetreloc.c

Index: test/CodeGen/bpf-offsetreloc.c
===
--- /dev/null
+++ test/CodeGen/bpf-offsetreloc.c
@@ -0,0 +1,17 @@
+// RUN: %clang -target bpf -emit-llvm -S -Xclang -target-feature -Xclang +offsetreloc -o - %s > %t1
+// RUN: grep "llvm.preserve.struct.access.index" %t1
+// RUN: grep "llvm.preserve.array.access.index" %t1
+// RUN: grep "llvm.preserve.union.access.index" %t1
+
+struct t {
+  int i:1;
+  int j:2;
+  union {
+   int a;
+   int b;
+  } c[4];
+};
+
+void *test(struct t *arg) {
+  return &arg->c[3].b;
+}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3350,8 +3350,17 @@
   CharUnits eltAlign =
 getArrayElementAlign(addr.getAlignment(), indices.back(), eltSize);
 
-  llvm::Value *eltPtr = emitArraySubscriptGEP(
-  CGF, addr.getPointer(), indices, inbounds, signedIndices, loc, name);
+  llvm::Value *eltPtr;
+  if (!CGF.getTarget().isPreserveDIAccessIndexNeeded()) {
+eltPtr = emitArraySubscriptGEP(
+CGF, addr.getPointer(), indices, inbounds, signedIndices, loc, name);
+  } else {
+// Remember the original array subscript for bpf target
+unsigned idx = cast(indices.back())->getZExtValue();
+eltPtr = CGF.Builder.CreatePreserveArrayAccessIndex(addr.getPointer(),
+name.str(), idx);
+  }
+
   return Address(eltPtr, eltAlign);
 }
 
@@ -3825,6 +3834,17 @@
   return CGF.Builder.CreateStructGEP(base, idx, field->getName());
 }
 
+static Address emitPreserveStructAccess(CodeGenFunction &CGF, Address base,
+const FieldDecl *field) {
+  const RecordDecl *rec = field->getParent();
+
+  unsigned idx =
+  CGF.CGM.getTypes().getCGRecordLayout(rec).getLLVMFieldNo(field);
+
+  return CGF.Builder.CreatePreserveStructAccessIndex(
+  base, field->getName(), rec->getName(), idx, field->getFieldIndex());
+}
+
 static bool hasAnyVptr(const QualType Type, const ASTContext &Context) {
   const auto *RD = Type.getTypePtr()->getAsCXXRecordDecl();
   if (!RD)
@@ -3932,9 +3952,21 @@
   // a barrier every time CXXRecord field with vptr is referenced.
   addr = Address(Builder.CreateLaunderInvariantGroup(addr.getPointer()),
  addr.getAlignment());
+
+if (getTarget().isPreserveDIAccessIndexNeeded())
+  // Remember the original union field index
+  addr = Address(
+  Builder.CreatePreserveUnionAccessIndex(
+  addr.getPointer(), rec->getName(), field->getFieldIndex()),
+  addr.getAlignment());
   } else {
-// For structs, we GEP to the field that the record layout suggests.
-addr = emitAddrOfFieldStorage(*this, addr, field);
+
+if (!getTarget().isPreserveDIAccessIndexNeeded())
+  // For structs, we GEP to the field that the record layout suggests.
+  addr = emitAddrOfFieldStorage(*this, addr, field);
+else
+  // Remember the original struct field index
+  addr = emitPreserveStructAccess(*this, addr, field);
 
 // If this is a reference field, load the reference right now.
 if (FieldType->isReferenceType()) {
Index: lib/CodeGen/CGBuilder.h
===
--- lib/CodeGen/CGBuilder.h
+++ lib/CodeGen/CGBuilder.h
@@ -298,6 +298,23 @@
 return CreateMemSet(Dest.getPointer(), Value, Size,
 Dest.getAlignment().getQuantity(), IsVolatile);
   }
+
+  using CGBuilderBaseTy::CreatePreserveStructAccessIndex;
+  Address CreatePreserveStructAccessIndex(Address Addr,
+  const llvm::StringRef &InstName,
+  const llvm::StringRef &StructName,
+  unsigned Index,
+  unsigned FieldIndex) {
+llvm::StructType *ElTy = cast(Addr.getElementType());
+const llvm::DataLayout &DL = BB->getParent()->getParent()->getDataLayout();
+const llvm::StructLayout *Layout = DL.getStructLayout(ElTy);
+auto Offset = CharUnits::fromQuantity(Layout->getElementOffset(Index));
+
+return Address(CreatePreserveStructAccessIndex(Add

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

LG, just a few NITs wrt to exposing implementation details in the header.




Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:161
+// components are gathered as a `Case` and rules are defined as an ordered list
+// of cases.
 //

NIT: maybe clarify what "ordered" means? E.g. "The first rule that matches gets 
applied and the rest are ignored"



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:278
+/// Builds the matcher needed for registration.
+ast_matchers::internal::DynTypedMatcher buildMatcher(const RewriteRule &Rule);
+

Can it be declared in `.cpp` file instead? Or is it used in `clang-tidy` 
integration? 



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:282
+const RewriteRule::Case &
+findSelectedCase(const ast_matchers::MatchFinder::MatchResult &Result,
+ const RewriteRule &Rule);

Same here, so far this looks like an implementation detail of `Transformer`, 
why not put it into `.cpp` file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61335



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

[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type name/access index

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

@rsmith @eli.friedman Hi, I just implemented a new version which generates 
intrinsics only. Three intrinsics are implemented for array/union/structure 
separately as their handling and parameters are different. Could you take a 
look? Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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

Re: r360308 - [c++20] Implement P0846R0: allow (ADL-only) calls to template-ids whose

[Richard Smith via cfe-commits]

On Thu, 16 May 2019, 03:38 Stephan Bergmann via cfe-commits, <
cfe-commits@lists.llvm.org> wrote:

> On 15/05/2019 18:16, Stephan Bergmann wrote:
> > The below commit started to cause the following failure:
>
> ...and which apparently got fixed in the meantime, presumably by
> <
> https://github.com/llvm/llvm-project/commit/beda951d788a0041e9f5fabbb4e018e8b9d0a2d3>
>
> "Make tentative parsing to detect template-argument-lists less aggressive"
>

Yes, sorry, I forgot to reply here after fixing it!

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

[PATCH] D61970: [CodeGen][ObjC] Call objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue instead of objc_autorelease and objc_retain in MRR

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/Driver/Options.td:1500
+  Flag<["-"], "fobjc-no-builtin-retain-release">, Group,
+  Flags<[CC1Option]>;
 def fno_objc_convert_messages_to_runtime_calls :

I know I suggested this name, but it should probably be `-fno-objc-` 
instead of `-fobjc-no-`.

Please add help text.  Also, there probably ought to be a subsection in the 
user docs under "Objective-C Features" which talks about message rewriting; it 
should start by discussing the general fact that we rewrite messages, including 
how to disable it with `-fno-objc-convert-messages-to-runtime-calls`, and then 
segue into the special assumptions for retain/release/autorelease and how to 
disable them with this option.

Should this be implied by `-fno-objc-convert-messages-to-runtime-calls`?



Comment at: lib/CodeGen/CGObjC.cpp:2597
+  if (!CGM.getCodeGenOpts().ObjCNoBuiltinRetainRelease && !getInvokeDest())
+return EmitARCAutoreleaseReturnValue(value, returnType);
   return emitObjCValueOperation(

Is using autoreleaseRV in all situations a good idea?  Among other things, 
won't this make it significantly more difficult to detect miscompiles where we 
fail to tail call autoreleaseRV?



Comment at: lib/CodeGen/CGObjC.cpp:2611
+  if (!CGM.getCodeGenOpts().ObjCNoBuiltinRetainRelease && !getInvokeDest())
+return EmitARCRetainAutoreleasedReturnValue(value, returnType);
   return emitObjCValueOperation(

Same question: won't this make it significantly more difficult to detect when 
we're generating bad code?  And the overhead here (on every retain in MRC!) is 
pretty significant; shouldn't this at least be restricted to situations where 
it's plausible that it might be reclaiming something, like when the receiver 
expression is a function call of some sort?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61970



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

[PATCH] D59879: [ARM][CMSE] Add commandline option and feature macro

[Dave Green via Phabricator via cfe-commits]

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

LGTM. With one comment that I will leave to you for what you think is best.




Comment at: include/clang/Driver/Options.td:2145
+  Flags<[DriverOption,CC1Option]>,
+  HelpText<"Allow use of CMSE instructions (ARM only)">;
 

dmgreen wrote:
> Should this say something about compiling for secure state?
Maybe "Enables generation of secure code for CMSE (Armv8-M Security 
Extensions)".

I meant to say that non-secure code can still be compiled without this flag, 
and can use some cmse features (as far as I understand). Its the secure side 
that needs this flag to do the clearing of registers and whatnot.


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

https://reviews.llvm.org/D59879



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

[PATCH] D61989: [clang-tidy] enable modernize-concat-nested-namespaces on header files

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Hi shixiao,

thank you for taking a look at this. Please extend the unit-tests 
(clang-tools-extra/test/clang-tidy) for this check, your revision description 
already includes a good start.




Comment at: 
clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:69
+  "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
+  if (!utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+HeaderFileExtensions, ',')) {

please ellide braces for the single-stmt `if`s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61989



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

r360918 - [X86] Update doxygen comments for AVX512BF16 to not refer to masks as 'immediates'. Refer to parameter names instead of 'src', 'src1', 'src2'. NFC

[Craig Topper via cfe-commits]

Author: ctopper
Date: Thu May 16 10:34:35 2019
New Revision: 360918

URL: http://llvm.org/viewvc/llvm-project?rev=360918&view=rev
Log:
[X86] Update doxygen comments for AVX512BF16 to not refer to masks as 
'immediates'. Refer to parameter names instead of 'src', 'src1', 'src2'. NFC

Modified:
cfe/trunk/lib/Headers/avx512bf16intrin.h
cfe/trunk/lib/Headers/avx512vlbf16intrin.h

Modified: cfe/trunk/lib/Headers/avx512bf16intrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512bf16intrin.h?rev=360918&r1=360917&r2=360918&view=diff
==
--- cfe/trunk/lib/Headers/avx512bf16intrin.h (original)
+++ cfe/trunk/lib/Headers/avx512bf16intrin.h Thu May 16 10:34:35 2019
@@ -31,7 +31,7 @@ typedef short __m256bh __attribute__((__
 /// \param __B
 ///A 512-bit vector of [16 x float].
 /// \returns A 512-bit vector of [32 x bfloat] whose lower 256 bits come from
-///convertion of src2, and higher 256 bits come from conversion of src1.
+///conversion of __B, and higher 256 bits come from conversion of __A.
 static __inline__ __m512bh __DEFAULT_FN_ATTRS512
 _mm512_cvtne2ps_pbh(__m512 __A, __m512 __B) {
   return (__m512bh)__builtin_ia32_cvtne2ps2bf16_512((__v16sf) __A,
@@ -51,10 +51,10 @@ _mm512_cvtne2ps_pbh(__m512 __A, __m512 _
 /// \param __W
 ///A 512-bit vector of [32 x bfloat].
 /// \param __U
-///An immediate value containing an 32-bit value specifying which element
-///is choosed. 1 means __A or __B, 0 means __W.
+///A 32-bit mask value specifying what is chosen for each element.
+///A 1 means conversion of __A or __B. A 0 means element from __W.
 /// \returns A 512-bit vector of [32 x bfloat] whose lower 256 bits come from
-///convertion of src2, and higher 256 bits come from conversion of src1.
+///conversion of __B, and higher 256 bits come from conversion of __A.
 static __inline__ __m512bh __DEFAULT_FN_ATTRS512
 _mm512_mask_cvtne2ps_pbh(__m512bh __W, __mmask32 __U, __m512 __A, __m512 __B) {
   return (__m512bh)__builtin_ia32_selectw_512((__mmask32)__U,
@@ -73,10 +73,10 @@ _mm512_mask_cvtne2ps_pbh(__m512bh __W, _
 /// \param __B
 ///A 512-bit vector of [16 x float].
 /// \param __U
-///An immediate value containing an 32-bit value specifying which element
-///is choosed. 1 means __A or __B, 0 means zero.
+///A 32-bit mask value specifying what is chosen for each element.
+///A 1 means conversion of __A or __B. A 0 means element is zero.
 /// \returns A 512-bit vector of [32 x bfloat] whose lower 256 bits come from
-///convertion of src2, and higher 256 bits come from conversion of src1.
+///conversion of __B, and higher 256 bits come from conversion of __A.
 static __inline__ __m512bh __DEFAULT_FN_ATTRS512
 _mm512_maskz_cvtne2ps_pbh(__mmask32 __U, __m512 __A, __m512 __B) {
   return (__m512bh)__builtin_ia32_selectw_512((__mmask32)__U,
@@ -92,7 +92,7 @@ _mm512_maskz_cvtne2ps_pbh(__mmask32 __U,
 ///
 /// \param __A
 ///A 512-bit vector of [16 x float].
-/// \returns A 256-bit vector of [16 x bfloat] come from convertion of src
+/// \returns A 256-bit vector of [16 x bfloat] come from conversion of __A.
 static __inline__ __m256bh __DEFAULT_FN_ATTRS512
 _mm512_cvtneps_pbh(__m512 __A) {
   return (__m256bh)__builtin_ia32_cvtneps2bf16_512((__v16sf) __A);
@@ -109,9 +109,9 @@ _mm512_cvtneps_pbh(__m512 __A) {
 /// \param __W
 ///A 256-bit vector of [16 x bfloat].
 /// \param __U
-///An immediate value containing an 16-bit value specifying which element
-///is choosed. 1 means __A, 0 means __W.
-/// \returns A 256-bit vector of [16 x bfloat] come from convertion of src
+///A 16-bit mask value specifying what is chosen for each element.
+///A 1 means conversion of __A. A 0 means element from __W.
+/// \returns A 256-bit vector of [16 x bfloat] come from conversion of __A.
 static __inline__ __m256bh __DEFAULT_FN_ATTRS512
 _mm512_mask_cvtneps_pbh(__m256bh __W, __mmask16 __U, __m512 __A) {
   return (__m256bh)__builtin_ia32_selectw_256((__mmask16)__U,
@@ -128,9 +128,9 @@ _mm512_mask_cvtneps_pbh(__m256bh __W, __
 /// \param __A
 ///A 512-bit vector of [16 x float].
 /// \param __U
-///An immediate value containing an 16-bit value specifying which element
-///is choosed. 1 means __A, 0 means zero.
-/// \returns A 256-bit vector of [16 x bfloat] come from convertion of src
+///A 16-bit mask value specifying what is chosen for each element.
+///A 1 means conversion of __A. A 0 means element is zero.
+/// \returns A 256-bit vector of [16 x bfloat] come from conversion of __A.
 static __inline__ __m256bh __DEFAULT_FN_ATTRS512
 _mm512_maskz_cvtneps_pbh(__mmask16 __U, __m512 __A) {
   return (__m256bh)__builtin_ia32_selectw_256((__mmask16)__U,
@@ -172,8 +172,8 @@ _mm512_dpbf16_ps(__m512 __D, __m512bh __
 /// \param __D
 ///A 512-bit vector of [16 x float].
 /// \param __U
-///An immediate value containing an 16-bit v

[PATCH] D60456: [RISCV][WIP/RFC] Hard float ABI support

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:9232
+if (IsFloat && Size > FLen)
+  return false;
+// Can't be eligible if an integer type was already found (only fp+int or

Is this the only consideration for floating-point types?  Clang does have 
increasing support for `half` / various `float16` types.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:9236
+if (IsInt && Field1Ty && Field1Ty->isIntegerTy())
+  return false;
+if (!Field1Ty) {

The comment here is wrong because fp+fp is allowed, right?

Is this not already caught by the post-processing checks you do in 
`detectFPCCEligibleStruct`?  Would it make more sense to just do all those 
checks there?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:9258
+assert(CurOff.isZero() && "Unexpected offset for first field");
+Field2Ty = CGT.ConvertType(EltTy);
+Field2Off = getContext().getTypeSizeInChars(EltTy);

`Field2Ty = Field1Ty`, please.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:9288
+  return false;
+for (const FieldDecl *FD : RD->fields()) {
+  const ASTRecordLayout &Layout = getContext().getASTRecordLayout(RD);

I really expect there to be something in this block about whether the field is 
a bit-field.  What exactly does your ABI specify if there's a bit-field?


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

https://reviews.llvm.org/D60456



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

[PATCH] D61909: Add Clang shared library with C++ exports

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

There is a simpler example distribution configuration, but sadly there isn't 
documentation. That is something I can fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61909



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

[PATCH] D61909: Add Clang shared library with C++ exports

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

I should note, this and many more aspects of the build system were topics I 
discussed in a talk at the 2016 LLVM Dev meeting when we switched off autoconf:
https://www.youtube.com/watch?v=StF77Cx7pz8


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61909



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

[PATCH] D61835: Extract ASTDumper to a header file

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D61835#1504663 , @aaron.ballman 
wrote:

> I'm not certain where you're planning to go with this change (or is this the 
> only change you're trying to make in this area?), so it's a bit hard to 
> evaluate this patch. Can you explain a bit more about what you're ultimately 
> trying to accomplish?
>
> It might help if I had a better idea of which APIs you thought were ones that 
> would help users (because my only real concern with this change is that the 
> public interface for this class is rather unpleasant).


The reason the `ASTDumper` class still exists (for the purpose of dumping an 
AST to stream at least) is that it dumps the `{Function,Var,Class}TemplateDecl` 
'correctly'.

The users of the follow-up patch 
https://reviews.llvm.org/D61837#change-x5mxz9Lpijjs need that 'correctness', 
but also need the public API from `ASTNodeTraverser` on the instance. (That 
patch also extends the public API for users).

Perhaps some day the stream-dump output can be changed and the `ASTDumper` 
class will not be needed anymore. This is not that day :).


Repository:
  rC Clang

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

https://reviews.llvm.org/D61835



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

[PATCH] D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D61834#1504665 , @aaron.ballman 
wrote:

> What will be making use of/testing this new functionality?


Any code which has a `DynTypedNode` and wishes to traverse it.

I envisage this as a more-flexible `DynTypedNode::dump` that the user does not 
have to implement themselves in order to use the `ASTNodeTraverser`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61834



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

r360920 - Move TraversalKind enum to ast_type_traits

[Stephen Kelly via cfe-commits]

Author: steveire
Date: Thu May 16 10:57:38 2019
New Revision: 360920

URL: http://llvm.org/viewvc/llvm-project?rev=360920&view=rev
Log:
Move TraversalKind enum to ast_type_traits

Summary:
Make it usable outside of ASTMatchFinder.  This will make it possible to
use this enum to control whether certain implicit nodes are skipped
while AST dumping for example.

Reviewers: klimek, aaron.ballman

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/AST/ASTTypeTraits.h
cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersInternalTest.cpp

Modified: cfe/trunk/include/clang/AST/ASTTypeTraits.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTTypeTraits.h?rev=360920&r1=360919&r2=360920&view=diff
==
--- cfe/trunk/include/clang/AST/ASTTypeTraits.h (original)
+++ cfe/trunk/include/clang/AST/ASTTypeTraits.h Thu May 16 10:57:38 2019
@@ -38,6 +38,16 @@ struct PrintingPolicy;
 
 namespace ast_type_traits {
 
+/// Defines how we descend a level in the AST when we pass
+/// through expressions.
+enum TraversalKind {
+  /// Will traverse any child nodes.
+  TK_AsIs,
+
+  /// Will not traverse implicit casts and parentheses.
+  TK_IgnoreImplicitCastsAndParentheses
+};
+
 /// Kind identifier.
 ///
 /// It can be constructed from any node kind and allows for runtime type

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h?rev=360920&r1=360919&r2=360920&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h Thu May 16 
10:57:38 2019
@@ -950,15 +950,6 @@ const bool IsBaseType::value;
 /// all nodes, as all nodes have ancestors.
 class ASTMatchFinder {
 public:
-  /// Defines how we descend a level in the AST when we pass
-  /// through expressions.
-  enum TraversalKind {
-/// Will traverse any child nodes.
-TK_AsIs,
-
-/// Will not traverse implicit casts and parentheses.
-TK_IgnoreImplicitCastsAndParentheses
-  };
 
   /// Defines how bindings are processed on recursive matches.
   enum BindKind {
@@ -989,11 +980,9 @@ public:
   BoundNodesTreeBuilder *Builder) = 0;
 
   template 
-  bool matchesChildOf(const T &Node,
-  const DynTypedMatcher &Matcher,
+  bool matchesChildOf(const T &Node, const DynTypedMatcher &Matcher,
   BoundNodesTreeBuilder *Builder,
-  TraversalKind Traverse,
-  BindKind Bind) {
+  ast_type_traits::TraversalKind Traverse, BindKind Bind) {
 static_assert(std::is_base_of::value ||
   std::is_base_of::value ||
   std::is_base_of::value ||
@@ -1042,7 +1031,7 @@ protected:
   virtual bool matchesChildOf(const ast_type_traits::DynTypedNode &Node,
   const DynTypedMatcher &Matcher,
   BoundNodesTreeBuilder *Builder,
-  TraversalKind Traverse,
+  ast_type_traits::TraversalKind Traverse,
   BindKind Bind) = 0;
 
   virtual bool matchesDescendantOf(const ast_type_traits::DynTypedNode &Node,
@@ -1290,7 +1279,7 @@ public:
   bool matches(const T &Node, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder) const override {
 return Finder->matchesChildOf(Node, this->InnerMatcher, Builder,
-  ASTMatchFinder::TK_AsIs,
+  ast_type_traits::TraversalKind::TK_AsIs,
   ASTMatchFinder::BK_First);
   }
 };
@@ -1313,7 +1302,7 @@ class ForEachMatcher : public WrapperMat
BoundNodesTreeBuilder* Builder) const override {
 return Finder->matchesChildOf(
 Node, this->InnerMatcher, Builder,
-ASTMatchFinder::TK_IgnoreImplicitCastsAndParentheses,
+ast_type_traits::TraversalKind::TK_IgnoreImplicitCastsAndParentheses,
 ASTMatchFinder::BK_All);
   }
 };

Modified: cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp?rev=360920&r1=360919&r2=360920&view=diff
==
--- cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp Thu May 16 10:57:38 2019
@@ -83,20 +83,12 @@ public:
   // descendants of a traversed node. max_depth is the maximum depth
   // to traverse: use 1 for matching the children and IN

[PATCH] D61836: Move TraversalKind enum to ast_type_traits

[Stephen Kelly via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC360920: Move TraversalKind enum to ast_type_traits (authored 
by steveire, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61836?vs=199180&id=199856#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D61836

Files:
  include/clang/AST/ASTTypeTraits.h
  include/clang/ASTMatchers/ASTMatchersInternal.h
  lib/ASTMatchers/ASTMatchFinder.cpp
  unittests/ASTMatchers/ASTMatchersInternalTest.cpp

Index: lib/ASTMatchers/ASTMatchFinder.cpp
===
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -83,20 +83,12 @@
   // descendants of a traversed node. max_depth is the maximum depth
   // to traverse: use 1 for matching the children and INT_MAX for
   // matching the descendants.
-  MatchChildASTVisitor(const DynTypedMatcher *Matcher,
-   ASTMatchFinder *Finder,
-   BoundNodesTreeBuilder *Builder,
-   int MaxDepth,
-   ASTMatchFinder::TraversalKind Traversal,
+  MatchChildASTVisitor(const DynTypedMatcher *Matcher, ASTMatchFinder *Finder,
+   BoundNodesTreeBuilder *Builder, int MaxDepth,
+   ast_type_traits::TraversalKind Traversal,
ASTMatchFinder::BindKind Bind)
-  : Matcher(Matcher),
-Finder(Finder),
-Builder(Builder),
-CurrentDepth(0),
-MaxDepth(MaxDepth),
-Traversal(Traversal),
-Bind(Bind),
-Matches(false) {}
+  : Matcher(Matcher), Finder(Finder), Builder(Builder), CurrentDepth(0),
+MaxDepth(MaxDepth), Traversal(Traversal), Bind(Bind), Matches(false) {}
 
   // Returns true if a match is found in the subtree rooted at the
   // given AST node. This is done via a set of mutually recursive
@@ -151,7 +143,8 @@
 
 ScopedIncrement ScopedDepth(&CurrentDepth);
 Stmt *StmtToTraverse = StmtNode;
-if (Traversal == ASTMatchFinder::TK_IgnoreImplicitCastsAndParentheses) {
+if (Traversal ==
+ast_type_traits::TraversalKind::TK_IgnoreImplicitCastsAndParentheses) {
   if (Expr *ExprNode = dyn_cast_or_null(StmtNode))
 StmtToTraverse = ExprNode->IgnoreParenImpCasts();
 }
@@ -299,7 +292,7 @@
   BoundNodesTreeBuilder ResultBindings;
   int CurrentDepth;
   const int MaxDepth;
-  const ASTMatchFinder::TraversalKind Traversal;
+  const ast_type_traits::TraversalKind Traversal;
   const ASTMatchFinder::BindKind Bind;
   bool Matches;
 };
@@ -393,7 +386,8 @@
   bool memoizedMatchesRecursively(const ast_type_traits::DynTypedNode &Node,
   const DynTypedMatcher &Matcher,
   BoundNodesTreeBuilder *Builder, int MaxDepth,
-  TraversalKind Traversal, BindKind Bind) {
+  ast_type_traits::TraversalKind Traversal,
+  BindKind Bind) {
 // For AST-nodes that don't have an identity, we can't memoize.
 if (!Node.getMemoizationData() || !Builder->isComparable())
   return matchesRecursively(Node, Matcher, Builder, MaxDepth, Traversal,
@@ -427,7 +421,8 @@
   bool matchesRecursively(const ast_type_traits::DynTypedNode &Node,
   const DynTypedMatcher &Matcher,
   BoundNodesTreeBuilder *Builder, int MaxDepth,
-  TraversalKind Traversal, BindKind Bind) {
+  ast_type_traits::TraversalKind Traversal,
+  BindKind Bind) {
 MatchChildASTVisitor Visitor(
   &Matcher, this, Builder, MaxDepth, Traversal, Bind);
 return Visitor.findMatch(Node);
@@ -441,7 +436,7 @@
   bool matchesChildOf(const ast_type_traits::DynTypedNode &Node,
   const DynTypedMatcher &Matcher,
   BoundNodesTreeBuilder *Builder,
-  TraversalKind Traversal,
+  ast_type_traits::TraversalKind Traversal,
   BindKind Bind) override {
 if (ResultCache.size() > MaxMemoizationEntries)
   ResultCache.clear();
@@ -456,7 +451,8 @@
 if (ResultCache.size() > MaxMemoizationEntries)
   ResultCache.clear();
 return memoizedMatchesRecursively(Node, Matcher, Builder, INT_MAX,
-  TK_AsIs, Bind);
+  ast_type_traits::TraversalKind::TK_AsIs,
+  Bind);
   }
   // Implements ASTMatchFinder::matchesAncestorOf.
   bool matchesAncestorOf(const ast_type_traits::DynTypedNode &Node,
Index: unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ unittests/ASTMatchers/A

r360922 - Update comments on enums

[Stephen Kelly via cfe-commits]

Author: steveire
Date: Thu May 16 11:02:36 2019
New Revision: 360922

URL: http://llvm.org/viewvc/llvm-project?rev=360922&view=rev
Log:
Update comments on enums

Modified:
cfe/trunk/include/clang/AST/ASTTypeTraits.h

Modified: cfe/trunk/include/clang/AST/ASTTypeTraits.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTTypeTraits.h?rev=360922&r1=360921&r2=360922&view=diff
==
--- cfe/trunk/include/clang/AST/ASTTypeTraits.h (original)
+++ cfe/trunk/include/clang/AST/ASTTypeTraits.h Thu May 16 11:02:36 2019
@@ -41,10 +41,11 @@ namespace ast_type_traits {
 /// Defines how we descend a level in the AST when we pass
 /// through expressions.
 enum TraversalKind {
-  /// Will traverse any child nodes.
+  /// Will traverse all child nodes.
   TK_AsIs,
 
   /// Will not traverse implicit casts and parentheses.
+  /// Corresponds to Expr::IgnoreParenImpCasts()
   TK_IgnoreImplicitCastsAndParentheses
 };
 


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

[PATCH] D61836: Move TraversalKind enum to ast_type_traits

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

Thanks, I made the comment changes in a separate commit because otherwise this 
would cease to be a refactoring commit.  Commits get too noisy if their content 
is modified "mid-flight".


Repository:
  rC Clang

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

https://reviews.llvm.org/D61836



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

[PATCH] D61909: Add Clang shared library with C++ exports

[Wink Saville via Phabricator via cfe-commits]

winksaville added a comment.

In D61909#1505046 , @beanz wrote:

> There is a simpler example distribution configuration, but sadly there isn't 
> documentation. That is something I can fix.


Please add documentation if you want people to use it :)
In the mean time I'll watch the the talk 
 you mentioned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61909



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

[PATCH] D61909: Add Clang shared library with C++ exports

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

In D61909#1505083 , @winksaville wrote:

> Please add documentation if you want people to use it :)


I won't disagree that it should be documented, and I already started. I do want 
to point out though that your motivation starting this was Arch linux where 
they are doing exactly what our documentation says not to do. So...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61909



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

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

[Jian Cai via Phabricator via cfe-commits]

jcai19 updated this revision to Diff 199861.
jcai19 added a comment.

Add fixes based on comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61967

Files:
  clang-tools-extra/clang-tidy/android/AndroidTidyModule.cpp
  clang-tools-extra/clang-tidy/android/CMakeLists.txt
  clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp
  clang-tools-extra/clang-tidy/android/CloexecPipeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp

Index: clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s android-cloexec-pipe %t
+
+extern "C" int pipe(int pipefd[2]);
+
+void f() {
+  int pipefd[2];
+  pipe(pipefd);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer pipe2() to pipe() because pipe2() allows O_CLOEXEC [android-cloexec-pipe]
+  // CHECK-FIXES: pipe2(pipefd, O_CLOEXEC);
+}
+
+namespace i {
+int pipe(int pipefd[2]);
+void g() {
+  int pipefd[2];
+  pipe(pipefd);
+}
+} // namespace i
+
+class C {
+public:
+  int pipe(int pipefd[2]);
+  void h() {
+int pipefd[2];
+pipe(pipefd);
+  }
+};
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -32,6 +32,7 @@
android-cloexec-inotify-init1
android-cloexec-memfd-create
android-cloexec-open
+   android-cloexec-pipe
android-cloexec-socket
android-comparison-in-temp-failure-retry
boost-use-to-string
Index: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - android-cloexec-pipe
+
+android-cloexec-pipe
+==
+
+The usage of ``pipe()`` is not recommended, it's better to use ``pipe2()``.
+Without this flag, an opened sensitive file descriptor would remain open across
+a fork+exec to a lower-privileged SELinux domain.
+
+Examples:
+
+.. code-block:: c++
+
+  pipe(pipefd);
+
+  // becomes
+
+  pipe2(pipefd, O_CLOEXEC);
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -101,6 +101,11 @@
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
   in the Time domain instead of the numeric domain.
 
+- New :doc:`android-cloexec-pipe
+  ` check.
+
+  Detects usage of ``pipe()``.
+
 - New :doc:`bugprone-unhandled-self-assignment
   ` check.
 
Index: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/android/CloexecPipeCheck.h
@@ -0,0 +1,34 @@
+//===--- CloexecPipeCheck.h - clang-tidy---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_PIPE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_PIPE_H
+
+#include "CloexecCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace android {
+
+/// pipe() is better to be replaced by pipe2().
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-pipe.html
+class CloexecPipeCheck : public CloexecCheck {
+public:
+  CloexecPipeCheck(StringRef Name, ClangTidyContext *Context)
+  : CloexecCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace android
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_PIPE_H
Index: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp
@@ -0,0 +1,37 @@
+//===--- CloexecPipeCheck.cpp - clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-Licens

[PATCH] D61909: Add Clang shared library with C++ exports

[Tom Stellard via Phabricator via cfe-commits]

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

LGTM.  We can add CLANG_LINK_CLANG_DYLIB as a follow-up patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61909



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

r360924 - [X86] Stop implicitly enabling avx512vl when avx512bf16 is enabled.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Thu May 16 11:28:17 2019
New Revision: 360924

URL: http://llvm.org/viewvc/llvm-project?rev=360924&view=rev
Log:
[X86] Stop implicitly enabling avx512vl when avx512bf16 is enabled.

Previously we were doing this so that the 256 bit selectw builtin could be used 
in the implementation of the 512->256 bit conversion intrinsic.

After this commit we now use a masked convert builtin that will emit the 
intrinsic call and the 256-bit select from custom code in CGBuiltin. Then the 
header only needs to call that one intrinsic.

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/Basic/Targets/X86.cpp
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/avx512bf16intrin.h
cfe/trunk/lib/Headers/avx512vlbf16intrin.h
cfe/trunk/test/Preprocessor/x86_target_features.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=360924&r1=360923&r2=360924&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Thu May 16 11:28:17 2019
@@ -1831,24 +1831,15 @@ TARGET_BUILTIN(__builtin_ia32_cvtusi2ss3
 TARGET_BUILTIN(__builtin_ia32_vpmultishiftqb512, "V64cV64cV64c", "ncV:512:", 
"avx512vbmi")
 TARGET_BUILTIN(__builtin_ia32_vpmultishiftqb128, "V16cV16cV16c", "ncV:128:", 
"avx512vbmi,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_vpmultishiftqb256, "V32cV32cV32c", "ncV:256:", 
"avx512vbmi,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_cvtne2ps2bf16_128, "V8sV4fV4f", "ncV:128:",
- "avx512bf16,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_cvtne2ps2bf16_256, "V16sV8fV8f", "ncV:256:",
- "avx512bf16,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_cvtne2ps2bf16_512, "V32sV16fV16f", "ncV:512:",
- "avx512bf16")
-TARGET_BUILTIN(__builtin_ia32_cvtneps2bf16_128_mask, "V8sV4fV8sUc", "ncV:128:",
-"avx512bf16,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_cvtneps2bf16_256, "V8sV8f", "ncV:256:",
-"avx512bf16,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_cvtneps2bf16_512, "V16sV16f", "ncV:512:",
-"avx512bf16")
-TARGET_BUILTIN(__builtin_ia32_dpbf16ps_128, "V4fV4fV4iV4i", "ncV:128:",
-"avx512bf16,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_dpbf16ps_256, "V8fV8fV8iV8i", "ncV:256:",
-"avx512bf16,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_dpbf16ps_512, "V16fV16fV16iV16i", "ncV:512:",
-"avx512bf16")
+TARGET_BUILTIN(__builtin_ia32_cvtne2ps2bf16_128, "V8sV4fV4f", "ncV:128:", 
"avx512bf16,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_cvtne2ps2bf16_256, "V16sV8fV8f", "ncV:256:", 
"avx512bf16,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_cvtne2ps2bf16_512, "V32sV16fV16f", "ncV:512:", 
"avx512bf16")
+TARGET_BUILTIN(__builtin_ia32_cvtneps2bf16_128_mask, "V8sV4fV8sUc", 
"ncV:128:", "avx512bf16,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_cvtneps2bf16_256_mask, "V8sV8fV8sUc", 
"ncV:256:", "avx512bf16,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_cvtneps2bf16_512_mask, "V16sV16fV16sUs", 
"ncV:512:", "avx512bf16")
+TARGET_BUILTIN(__builtin_ia32_dpbf16ps_128, "V4fV4fV4iV4i", "ncV:128:", 
"avx512bf16,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_dpbf16ps_256, "V8fV8fV8iV8i", "ncV:256:", 
"avx512bf16,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_dpbf16ps_512, "V16fV16fV16iV16i", "ncV:512:", 
"avx512bf16")
 
 // generic select intrinsics
 TARGET_BUILTIN(__builtin_ia32_selectb_128, "V16cUsV16cV16c", "ncV:128:", 
"avx512bw,avx512vl")

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=360924&r1=360923&r2=360924&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Thu May 16 11:28:17 2019
@@ -661,14 +661,12 @@ void X86TargetInfo::setFeatureEnabledImp
 if ((Name.startswith("avx512vbmi") || Name == "avx512bitalg") && Enabled)
   Features["avx512bw"] = true;
 if (Name == "avx512bf16" && Enabled)
-  Features["avx512bw"] = Features["avx512vl"] = true;
+  Features["avx512bw"] = true;
 // Also disable VBMI/VBMI2/BITALG if BWI is being disabled.
 if (Name == "avx512bw" && !Enabled)
   Features["avx512vbmi"] = Features["avx512vbmi2"] =
   Features["avx512bf16"] =
   Features["avx512bitalg"] = false;
-if (Name == "avx512vl" && !Enabled)
-  Features["avx512bf16"] = false;
   } else if (Name == "fma") {
 if (Enabled)
   setSSELevel(Features, AVX, Enabled);

Modified:

[PATCH] D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D61834#1505056 , @steveire wrote:

> In D61834#1504665 , @aaron.ballman 
> wrote:
>
> > What will be making use of/testing this new functionality?
>
>
> Any code which has a `DynTypedNode` and wishes to traverse it.
>
> I envisage this as a more-flexible `DynTypedNode::dump` that the user does 
> not have to implement themselves in order to use the `ASTNodeTraverser`.


Do we currently have any such code that's using this functionality, though? I'm 
mostly concerned that this is dead code with no testing, currently. The 
functionality itself seems reasonable enough and the code looks correct enough, 
so if this is part of a series of planned changes, that'd be good information 
to have for the review.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61834



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

[PATCH] D62019: [clang] Handle lrint/llrint builtins

[Adhemerval Zanella via Phabricator via cfe-commits]

zatrazz created this revision.
zatrazz added reviewers: efriedma, rengolin, javed.absar, huntergr, 
SjoerdMeijer, t.p.northover, echristo, evandro.
zatrazz added a project: clang.
Herald added subscribers: kristina, kristof.beyls.

As for other floating-point rounding builtins that can be optimized
when build with -fno-math-errno, this patch adds support for lrint
and llrint.  It currently only optimize for AArch64 backend.

This patch depends on https://reviews.llvm.org/D62017.


Repository:
  rC Clang

https://reviews.llvm.org/D62019

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins.c
  clang/test/CodeGen/math-builtins.c
  clang/test/CodeGen/math-libcalls.c

Index: clang/test/CodeGen/math-libcalls.c
===
--- clang/test/CodeGen/math-libcalls.c
+++ clang/test/CodeGen/math-libcalls.c
@@ -308,9 +308,9 @@
 
   llrint(f); llrintf(f);llrintl(f);
 
-// NO__ERRNO: declare i64 @llrint(double) [[READNONE]]
-// NO__ERRNO: declare i64 @llrintf(float) [[READNONE]]
-// NO__ERRNO: declare i64 @llrintl(x86_fp80) [[READNONE]]
+// NO__ERRNO: declare i64 @llvm.llrint.f64(double) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare i64 @llvm.llrint.f32(float) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare i64 @llvm.llrint.f80(x86_fp80) [[READNONE_INTRINSIC]]
 // HAS_ERRNO: declare i64 @llrint(double) [[NOT_READNONE]]
 // HAS_ERRNO: declare i64 @llrintf(float) [[NOT_READNONE]]
 // HAS_ERRNO: declare i64 @llrintl(x86_fp80) [[NOT_READNONE]]
@@ -371,9 +371,9 @@
 
   lrint(f);  lrintf(f); lrintl(f);
 
-// NO__ERRNO: declare i64 @lrint(double) [[READNONE]]
-// NO__ERRNO: declare i64 @lrintf(float) [[READNONE]]
-// NO__ERRNO: declare i64 @lrintl(x86_fp80) [[READNONE]]
+// NO__ERRNO: declare i64 @llvm.lrint.i64.f64(double) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare i64 @llvm.lrint.i64.f32(float) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare i64 @llvm.lrint.i64.f80(x86_fp80) [[READNONE_INTRINSIC]]
 // HAS_ERRNO: declare i64 @lrint(double) [[NOT_READNONE]]
 // HAS_ERRNO: declare i64 @lrintf(float) [[NOT_READNONE]]
 // HAS_ERRNO: declare i64 @lrintl(x86_fp80) [[NOT_READNONE]]
Index: clang/test/CodeGen/math-builtins.c
===
--- clang/test/CodeGen/math-builtins.c
+++ clang/test/CodeGen/math-builtins.c
@@ -353,9 +353,9 @@
 
   __builtin_llrint(f); __builtin_llrintf(f);__builtin_llrintl(f);
 
-// NO__ERRNO: declare i64 @llrint(double) [[READNONE]]
-// NO__ERRNO: declare i64 @llrintf(float) [[READNONE]]
-// NO__ERRNO: declare i64 @llrintl(x86_fp80) [[READNONE]]
+// NO__ERRNO: declare i64 @llvm.llrint.f64(double) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare i64 @llvm.llrint.f32(float) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare i64 @llvm.llrint.f80(x86_fp80) [[READNONE_INTRINSIC]]
 // HAS_ERRNO: declare i64 @llrint(double) [[NOT_READNONE]]
 // HAS_ERRNO: declare i64 @llrintf(float) [[NOT_READNONE]]
 // HAS_ERRNO: declare i64 @llrintl(x86_fp80) [[NOT_READNONE]]
@@ -416,9 +416,9 @@
 
   __builtin_lrint(f);  __builtin_lrintf(f); __builtin_lrintl(f);
 
-// NO__ERRNO: declare i64 @lrint(double) [[READNONE]]
-// NO__ERRNO: declare i64 @lrintf(float) [[READNONE]]
-// NO__ERRNO: declare i64 @lrintl(x86_fp80) [[READNONE]]
+// NO__ERRNO: declare i64 @llvm.lrint.i64.f64(double) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare i64 @llvm.lrint.i64.f32(float) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare i64 @llvm.lrint.i64.f80(x86_fp80) [[READNONE_INTRINSIC]]
 // HAS_ERRNO: declare i64 @lrint(double) [[NOT_READNONE]]
 // HAS_ERRNO: declare i64 @lrintf(float) [[NOT_READNONE]]
 // HAS_ERRNO: declare i64 @lrintl(x86_fp80) [[NOT_READNONE]]
Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -390,6 +390,15 @@
 
   resli = __builtin_lroundl (LD);
   // CHECK: call i64 @llvm.lround.i64.f80
+
+  resli = __builtin_lrintf (F);
+  // CHECK: call i64 @llvm.lrint.i64.f32
+
+  resli = __builtin_lrint (D);
+  // CHECK: call i64 @llvm.lrint.i64.f64
+
+  resli = __builtin_lrintl (LD);
+  // CHECK: call i64 @llvm.lrint.i64.f80
 }
 
 // __builtin_longjmp isn't supported on all platforms, so only test it on X86.
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -1742,6 +1742,27 @@
 case Builtin::BI__builtin_llroundl:
   return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::llround));
 
+case Builtin::BIlrint:
+case Builtin::BIlrintf:
+case Builtin::BIlrintl:
+case Builtin::BI__builtin_lrint:
+case Builtin::BI__builtin_lrintf:
+case Builtin::BI__builtin_lrintl: {
+  llvm::Type *ResultType = ConvertType(E->getType());
+  int Width = ResultType->getPrimitiveSizeInBits();
+  return RValue::get(emitUnaryBuiltin

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 199869.
ymandel added a comment.

adjust some API comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61335

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

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -116,7 +116,8 @@
 };
   }
 
-  void testRule(RewriteRule Rule, StringRef Input, StringRef Expected) {
+  template 
+  void testRule(R Rule, StringRef Input, StringRef Expected) {
 Transformer T(std::move(Rule), consumer());
 T.registerMatchers(&MatchFinder);
 compareSnippets(Expected, rewrite(Input));
@@ -147,7 +148,7 @@
  .bind(StringExpr)),
   callee(cxxMethodDecl(hasName("c_str")),
   change("REPLACED"));
-  R.Explanation = text("Use size() method directly on string.");
+  R.Cases[0].Explanation = text("Use size() method directly on string.");
   return R;
 }
 
@@ -375,6 +376,92 @@
Input, Expected);
 }
 
+TEST_F(TransformerTest, OrderedRuleUnrelated) {
+  StringRef Flag = "flag";
+  RewriteRule FlagRule = makeRule(
+  cxxMemberCallExpr(on(expr(hasType(cxxRecordDecl(
+hasName("proto::ProtoCommandLineFlag"
+   .bind(Flag)),
+unless(callee(cxxMethodDecl(hasName("GetProto"),
+  change(Flag, "PROTO"));
+
+  std::string Input = R"cc(
+proto::ProtoCommandLineFlag flag;
+int x = flag.foo();
+int y = flag.GetProto().foo();
+int f(string s) { return strlen(s.c_str()); }
+  )cc";
+  std::string Expected = R"cc(
+proto::ProtoCommandLineFlag flag;
+int x = PROTO.foo();
+int y = flag.GetProto().foo();
+int f(string s) { return REPLACED; }
+  )cc";
+
+  testRule(applyFirst({ruleStrlenSize(), FlagRule}), Input, Expected);
+}
+
+// Version of ruleStrlenSizeAny that inserts a method with a different name than
+// ruleStrlenSize, so we can tell their effect apart.
+RewriteRule ruleStrlenSizeDistinct() {
+  StringRef S;
+  return makeRule(
+  callExpr(callee(functionDecl(hasName("strlen"))),
+   hasArgument(0, cxxMemberCallExpr(
+  on(expr().bind(S)),
+  callee(cxxMethodDecl(hasName("c_str")),
+  change("DISTINCT"));
+}
+
+TEST_F(TransformerTest, OrderedRuleRelated) {
+  std::string Input = R"cc(
+namespace foo {
+struct mystring {
+  char* c_str();
+};
+int f(mystring s) { return strlen(s.c_str()); }
+}  // namespace foo
+int g(string s) { return strlen(s.c_str()); }
+  )cc";
+  std::string Expected = R"cc(
+namespace foo {
+struct mystring {
+  char* c_str();
+};
+int f(mystring s) { return DISTINCT; }
+}  // namespace foo
+int g(string s) { return REPLACED; }
+  )cc";
+
+  testRule(applyFirst({ruleStrlenSize(), ruleStrlenSizeDistinct()}), Input,
+   Expected);
+}
+
+// Change the order of the rules to get a different result.
+TEST_F(TransformerTest, OrderedRuleRelatedSwapped) {
+  std::string Input = R"cc(
+namespace foo {
+struct mystring {
+  char* c_str();
+};
+int f(mystring s) { return strlen(s.c_str()); }
+}  // namespace foo
+int g(string s) { return strlen(s.c_str()); }
+  )cc";
+  std::string Expected = R"cc(
+namespace foo {
+struct mystring {
+  char* c_str();
+};
+int f(mystring s) { return DISTINCT; }
+}  // namespace foo
+int g(string s) { return DISTINCT; }
+  )cc";
+
+  testRule(applyFirst({ruleStrlenSizeDistinct(), ruleStrlenSize()}), Input,
+   Expected);
+}
+
 //
 // Negative tests (where we expect no transformation to occur).
 //
Index: clang/lib/Tooling/Refactoring/Transformer.cpp
===
--- clang/lib/Tooling/Refactoring/Transformer.cpp
+++ clang/lib/Tooling/Refactoring/Transformer.cpp
@@ -28,6 +28,7 @@
 using namespace tooling;
 
 using ast_matchers::MatchFinder;
+using ast_matchers::internal::DynTypedMatcher;
 using ast_type_traits::ASTNodeKind;
 using ast_type_traits::DynTypedNode;
 using llvm::Error;
@@ -144,9 +145,9 @@
   llvm_unreachable("Unexpected case in NodePart type.");
 }
 
-Expected>
-tooling::translateEdits(const MatchResult &Result,
-llvm::ArrayRef Edits) {
+Expected>
+tooling::detail::translateEdits(const MatchResult &Result,
+llvm::ArrayRef Edits) {
   SmallVector Transformations;
   auto &NodesMap = Result.Nodes.getMap();
   for (const auto &Edit : Edits) {
@@ -171,18 +172,113 @@
   return Transformations;
 }
 
-R

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

[Jian Cai via Phabricator via cfe-commits]

jcai19 updated this revision to Diff 199870.
jcai19 marked 4 inline comments as done.
jcai19 added a comment.

Fix format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61967

Files:
  clang-tools-extra/clang-tidy/android/AndroidTidyModule.cpp
  clang-tools-extra/clang-tidy/android/CMakeLists.txt
  clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp
  clang-tools-extra/clang-tidy/android/CloexecPipeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp

Index: clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s android-cloexec-pipe %t
+
+extern "C" int pipe(int pipefd[2]);
+
+void f() {
+  int pipefd[2];
+  pipe(pipefd);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer pipe2() to pipe() because pipe2() allows O_CLOEXEC [android-cloexec-pipe]
+  // CHECK-FIXES: pipe2(pipefd, O_CLOEXEC);
+}
+
+namespace i {
+int pipe(int pipefd[2]);
+void g() {
+  int pipefd[2];
+  pipe(pipefd);
+}
+} // namespace i
+
+class C {
+public:
+  int pipe(int pipefd[2]);
+  void h() {
+int pipefd[2];
+pipe(pipefd);
+  }
+};
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -32,6 +32,7 @@
android-cloexec-inotify-init1
android-cloexec-memfd-create
android-cloexec-open
+   android-cloexec-pipe
android-cloexec-socket
android-comparison-in-temp-failure-retry
boost-use-to-string
Index: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - android-cloexec-pipe
+
+android-cloexec-pipe
+
+
+Detects usage of ``pipe()``. The usage of ``pipe()`` is not recommended, it's better to use ``pipe2()``.
+Without this flag, an opened sensitive file descriptor would remain open across
+a ``fork``+``exec`` to a lower-privileged SELinux domain.
+
+Examples:
+
+.. code-block:: c++
+
+  pipe(pipefd);
+
+  // becomes
+
+  pipe2(pipefd, O_CLOEXEC);
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -101,6 +101,11 @@
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
   in the Time domain instead of the numeric domain.
 
+- New :doc:`android-cloexec-pipe
+  ` check.
+
+  Detects usage of ``pipe()``.
+
 - New :doc:`bugprone-unhandled-self-assignment
   ` check.
 
Index: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/android/CloexecPipeCheck.h
@@ -0,0 +1,34 @@
+//===--- CloexecPipeCheck.h - clang-tidy---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_PIPE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_PIPE_H
+
+#include "CloexecCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace android {
+
+/// pipe() is better to be replaced by pipe2().
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-pipe.html
+class CloexecPipeCheck : public CloexecCheck {
+public:
+  CloexecPipeCheck(StringRef Name, ClangTidyContext *Context)
+  : CloexecCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace android
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_PIPE_H
Index: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp
@@ -0,0 +1,37 @@
+//===--- CloexecPipeCheck.cpp - clang-tidy---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://ll

[PATCH] D61835: Extract ASTDumper to a header file

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D61835#1505048 , @steveire wrote:

> In D61835#1504663 , @aaron.ballman 
> wrote:
>
> > I'm not certain where you're planning to go with this change (or is this 
> > the only change you're trying to make in this area?), so it's a bit hard to 
> > evaluate this patch. Can you explain a bit more about what you're 
> > ultimately trying to accomplish?
> >
> > It might help if I had a better idea of which APIs you thought were ones 
> > that would help users (because my only real concern with this change is 
> > that the public interface for this class is rather unpleasant).
>
>
> The reason the `ASTDumper` class still exists (for the purpose of dumping an 
> AST to stream at least) is that it dumps the 
> `{Function,Var,Class}TemplateDecl` 'correctly'.


Yup, this I recall.

> The users of the follow-up patch 
> https://reviews.llvm.org/D61837#change-x5mxz9Lpijjs need that 'correctness', 
> but also need the public API from `ASTNodeTraverser` on the instance. (That 
> patch also extends the public API for users).

I don't see anything in the follow-up patch that actually uses the `ASTDumper` 
class though, so I'm still a bit confused.

> Perhaps some day the stream-dump output can be changed and the `ASTDumper` 
> class will not be needed anymore. This is not that day :).

Yeah, and I'm not suggesting today needs to be that day, either. :-) It's just 
that this looks like a change with no positive impact because the change isn't 
being used anywhere (that I can tell), so the only thing I have to go on is 
"does this produce a decent public API?" and my feeling is "no, this class is 
an implementation detail and should remain hidden", especially since we think 
we can remove it someday.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61835



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

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

[Jian Cai via Phabricator via cfe-commits]

jcai19 marked 4 inline comments as done.
jcai19 added inline comments.



Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:27
+void CloexecPipeCheck::check(const MatchFinder::MatchResult &Result) {
+  const std::string &ReplacementText =
+  (Twine("pipe2(") + getSpellingArg(Result, 0) + ", O_CLOEXEC)").str();

george.burgess.iv wrote:
> simplicity nit: can this be a `std::string`?
replaceFunc takes a llvm::StringRef as the third parameter. StringRef is 
"expected to be used in situations where the character data resides in some 
other buffer, whose lifetime extends past that of the StringRef", which is true 
in this case, so I think it should be fine.



Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.h:18
+
+/// accept() is better to be replaced by accept4().
+///

george.burgess.iv wrote:
> nit: should probably update this comment
Good catch!



Comment at: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst:4
+android-cloexec-pipe
+==
+

Eugene.Zelenko wrote:
> Please make same length as name above.
Thanks for the comments. I have fixed them accordingly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61967



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

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

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

rsmith added a comment.

In D58920#1503872 , @modocache wrote:

> Thanks for the help, @rsmith! Your suggestions were spot-on. (It took me a 
> little while to figure out why, even using the `LazyDeclPtr` directly, I was 
> still triggering deserialization. It turns out `dump()` causes 
> deserialization too -- whoops!)
>
> > You should also change `FindExistingResult::~FindExistingResult` to update 
> > `Sema::StdNamespace` to point to your newly-deserialized namespace if you 
> > didn't find a prior declaration of it, so that `Sema::getStdNamespace()` 
> > finds the deserialized namespace.
>
> I haven't done this yet. I'm trying to think of a test case that would fail 
> if this were not done properly -- or would there not be one?


I think the failure case is a little complex to set up. I think you need:

- two modules each of which contains an invisible (implicitly-declared) `std` 
namespace
- arrange for `Sema::StdNamespace` to point to the ID of one of them
- trigger the import of the other one
- perform a name lookup for `std` to erase the placeholder `std` lookup result 
from the translation unit's lookup table
- trigger the import of the first `std` (eg, by triggering the declaration of 
`operator new`)

At that point, there is nowhere for the newly-imported `std` namespace to find 
the old one: it's not in the translation unit's name lookup table, and it's not 
in `Sema::StdNamespace`, so we would presumably end up with two `std` 
namespaces not connected to one another.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58920



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

[PATCH] D36357: Added a better diagnostic when using the delete operator with lambdas

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

rsmith accepted this revision.
rsmith added a comment.

LGTM, thank you! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D36357



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

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 5 inline comments as done.
ymandel added a comment.

Thanks!!




Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:278
+/// Builds the matcher needed for registration.
+ast_matchers::internal::DynTypedMatcher buildMatcher(const RewriteRule &Rule);
+

ilya-biryukov wrote:
> Can it be declared in `.cpp` file instead? Or is it used in `clang-tidy` 
> integration? 
`buildMatcher` and `findSelectedCase` will be used in the clang-tidy 
integration and in the apply-rule-to-single-node function that I'm planning.



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:282
+const RewriteRule::Case &
+findSelectedCase(const ast_matchers::MatchFinder::MatchResult &Result,
+ const RewriteRule &Rule);

ilya-biryukov wrote:
> Same here, so far this looks like an implementation detail of `Transformer`, 
> why not put it into `.cpp` file?
see above. I think these also make sense as part of the RewriteRule 
abstraction. that is, you can think of these as methods of RewriteRule and they 
make sense even without thinking about transformer.  That said, if you think 
there's a way to make this clearer, i"m happy to adjust the comments/name, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61335



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

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

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

rsmith accepted this revision.
rsmith marked an inline comment as done.
rsmith added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM!




Comment at: clang/lib/Lex/Pragma.cpp:370
   // Push the tokens onto the stack.
-  EnterTokenStream(TokArray, PragmaToks.size(), true, true);
+  EnterTokenStream(TokArray, PragmaToks.size(), true, true,
+   /*IsReinject*/ false);

ilya-biryukov wrote:
> rsmith wrote:
> > I think this case is a reinjection; we've copied some tokens inside 
> > __pragma out into a duplicate position in the token stream. But I guess it 
> > doesn't matter because the tokens never escape the outer Lex function 
> > anyway.
> Yeah, my logic is that it's not a re-injection in the sense that they were 
> never the phase 4 tokens before.
Yeah, I think you're right. As a weird example, if we had a

#pragma clang produce_token x

pragma that just produced the given token, it'd be reasonable for it to leave 
the `Reinjected` flag alone, and it should come out as `false` in this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59885



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

[PATCH] D59464: [analyzer] Add an example plugin for checker dependency handling

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

All our bots are failing which seems to have been caused by the latest reland:

  FAIL: Clang :: Analysis/checker-plugins.c (297 of 14793)
   TEST 'Clang :: Analysis/checker-plugins.c' FAILED 

  Script:
  --
  : 'RUN: at line 1';   
/b/s/w/ir/k/recipe_cleanup/clangsCGbgY/llvm_build_dir/bin/clang -cc1 
-internal-isystem 
/b/s/w/ir/k/recipe_cleanup/clangsCGbgY/llvm_build_dir/lib/clang/9.0.0/include 
-nostdsysteminc -analyze -analyzer-constraints=range -verify 
/b/s/w/ir/k/llvm-project/clang/test/Analysis/checker-plugins.c-load 
/b/s/w/ir/k/recipe_cleanup/clangsCGbgY/llvm_build_dir/./lib/SampleAnalyzerPlugin.so
-analyzer-checker='example.MainCallChecker'
  : 'RUN: at line 15';   
/b/s/w/ir/k/recipe_cleanup/clangsCGbgY/llvm_build_dir/bin/clang -cc1 
-internal-isystem 
/b/s/w/ir/k/recipe_cleanup/clangsCGbgY/llvm_build_dir/lib/clang/9.0.0/include 
-nostdsysteminc -analyze -analyzer-constraints=range 
/b/s/w/ir/k/llvm-project/clang/test/Analysis/checker-plugins.c-load 
/b/s/w/ir/k/recipe_cleanup/clangsCGbgY/llvm_build_dir/./lib/CheckerDependencyHandlingAnalyzerPlugin.so
   -analyzer-checker=example.DependendentChecker
-analyzer-list-enabled-checkers2>&1 | 
/b/s/w/ir/k/recipe_cleanup/clangsCGbgY/llvm_build_dir/bin/FileCheck 
/b/s/w/ir/k/llvm-project/clang/test/Analysis/checker-plugins.c 
-check-prefix=CHECK-IMPLICITLY-ENABLED
  : 'RUN: at line 24';   
/b/s/w/ir/k/recipe_cleanup/clangsCGbgY/llvm_build_dir/bin/clang -cc1 
-internal-isystem 
/b/s/w/ir/k/recipe_cleanup/clangsCGbgY/llvm_build_dir/lib/clang/9.0.0/include 
-nostdsysteminc -analyze -analyzer-constraints=range 
/b/s/w/ir/k/llvm-project/clang/test/Analysis/checker-plugins.c-load 
/b/s/w/ir/k/recipe_cleanup/clangsCGbgY/llvm_build_dir/./lib/CheckerDependencyHandlingAnalyzerPlugin.so
   -analyzer-checker=example.DependendentChecker
-analyzer-disable-checker=example.Dependency-analyzer-list-enabled-checkers 
   2>&1 | /b/s/w/ir/k/recipe_cleanup/clangsCGbgY/llvm_build_dir/bin/FileCheck 
/b/s/w/ir/k/llvm-project/clang/test/Analysis/checker-plugins.c 
-check-prefix=CHECK-IMPLICITLY-DISABLED
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  error: 'error' diagnostics seen but not expected: 
(frontend): unable to load plugin 
'/b/s/w/ir/k/recipe_cleanup/clangsCGbgY/llvm_build_dir/./lib/SampleAnalyzerPlugin.so':
 
'/b/s/w/ir/k/recipe_cleanup/clangsCGbgY/llvm_build_dir/./lib/SampleAnalyzerPlugin.so:
 undefined symbol: _ZN4llvm14FoldingSetBase6anchorEv'
  
  --
  
  
  Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
  Testing Time: 77.86s
  
  Failing Tests (1):
  Clang :: Analysis/checker-plugins.c

Is it possible to revert this change again?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59464



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

[PATCH] D61835: Extract ASTDumper to a header file

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D61835#1505151 , @aaron.ballman 
wrote:

> In D61835#1505048 , @steveire wrote:
>
> > The users of the follow-up patch 
> > https://reviews.llvm.org/D61837#change-x5mxz9Lpijjs need that 
> > 'correctness', but also need the public API from `ASTNodeTraverser` on the 
> > instance. (That patch also extends the public API for users).
>
>
> I don't see anything in the follow-up patch that actually uses the 
> `ASTDumper` class though, so I'm still a bit confused.


Maybe my wording was confusing. I'll try again:

1. Anyone who wants traversal in the same way that dumping is done must use an 
instance of the `ASTDumper` class. Is that much clear? Does my previous 
response clarify why that is the case?

2. The follow-up patch https://reviews.llvm.org/D61837#change-x5mxz9Lpijjs adds 
API to ASTNodeTraverser.

3. Anyone who wants traversal in the same way that dumping is done and who 
needs to call API on the instance which is provided by ASTNodeTraverser (which 
ASTDumper inherits) needs to use ASTDumper. For example my UI. See my EuroLLVM 
talk for more: 
https://steveire.wordpress.com/2019/04/30/the-future-of-ast-matching-refactoring-tools-eurollvm-and-accu/


Repository:
  rC Clang

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

https://reviews.llvm.org/D61835



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

[PATCH] D62009: [clang] perform semantic checking in constant context

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

rsmith added a comment.

The approach we've been taking for this up until now is to use a `ConstantExpr` 
node to mark the entry point of a constant context; it looks like that would 
continue to work here, but we're missing those nodes in some cases? (This is 
preferable to using a flag because it also -- eventually -- gives us a place to 
store the evaluated value, which we're going to need for various upcoming 
features, particularly `consteval`.)


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

https://reviews.llvm.org/D62009



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

[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 199875.
steveire marked 10 inline comments as done.
steveire added a comment.

Update


Repository:
  rC Clang

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

https://reviews.llvm.org/D61837

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/ASTNodeTraverser.h
  include/clang/ASTMatchers/ASTMatchers.h
  include/clang/ASTMatchers/ASTMatchersInternal.h
  lib/AST/ASTContext.cpp
  lib/ASTMatchers/ASTMatchFinder.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1510,6 +1510,72 @@
   notMatches("class C {}; C a = C();", varDecl(has(cxxConstructExpr();
 }
 
+TEST(Traversal, traverseMatcher) {
+
+  StringRef VarDeclCode = R"cpp(
+void foo()
+{
+  int i = 3.0;
+}
+)cpp";
+
+  auto Matcher = varDecl(hasInitializer(floatLiteral()));
+
+  EXPECT_TRUE(
+  notMatches(VarDeclCode, traverse(ast_type_traits::TK_AsIs, Matcher)));
+  EXPECT_TRUE(
+  matches(VarDeclCode,
+  traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+   Matcher)));
+}
+
+TEST(Traversal, traverseMatcherNesting) {
+
+  StringRef Code = R"cpp(
+float bar(int i)
+{
+  return i;
+}
+
+void foo()
+{
+  bar(bar(3.0));
+}
+)cpp";
+
+  EXPECT_TRUE(matches(
+  Code,
+  traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+   callExpr(has(callExpr(traverse(
+   ast_type_traits::TK_AsIs,
+   callExpr(has(implicitCastExpr(has(floatLiteral(;
+}
+
+TEST(Traversal, traverseMatcherThroughMemoization) {
+
+  StringRef Code = R"cpp(
+void foo()
+{
+  int i = 3.0;
+}
+  )cpp";
+
+  auto Matcher = varDecl(hasInitializer(floatLiteral()));
+
+  // Matchers such as hasDescendant memoize their result regarding AST
+  // nodes. In the matcher below, the first use of hasDescendant(Matcher)
+  // fails, and the use of it inside the traverse() matcher should pass
+  // causing the overall matcher to be a true match.
+  // This test verifies that the first false result is not re-used, which
+  // would cause the overall matcher to be incorrectly false.
+
+  EXPECT_TRUE(matches(
+  Code, functionDecl(anyOf(
+hasDescendant(Matcher),
+traverse(ast_type_traits::TK_IgnoreImplicitCastsAndParentheses,
+ functionDecl(hasDescendant(Matcher)));
+}
+
 TEST(IgnoringImpCasts, MatchesImpCasts) {
   // This test checks that ignoringImpCasts matches when implicit casts are
   // present and its inner matcher alone does not match.
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -211,10 +211,19 @@
 bool DynTypedMatcher::matches(const ast_type_traits::DynTypedNode &DynNode,
   ASTMatchFinder *Finder,
   BoundNodesTreeBuilder *Builder) const {
-  if (RestrictKind.isBaseOf(DynNode.getNodeKind()) &&
-  Implementation->dynMatches(DynNode, Finder, Builder)) {
+  auto PreviousTraversalKind = Finder->getASTContext().GetTraversalKind();
+  auto OptTK = Implementation->TraversalKind();
+  if (OptTK)
+Finder->getASTContext().SetTraversalKind(*OptTK);
+  auto N = Finder->getASTContext().TraverseIgnored(DynNode);
+  auto NodeKind = N.getNodeKind();
+
+  if (RestrictKind.isBaseOf(NodeKind) &&
+  Implementation->dynMatches(N, Finder, Builder)) {
+Finder->getASTContext().SetTraversalKind(PreviousTraversalKind);
 return true;
   }
+  Finder->getASTContext().SetTraversalKind(PreviousTraversalKind);
   // Delete all bindings when a matcher does not match.
   // This prevents unexpected exposure of bound nodes in unmatches
   // branches of the match tree.
@@ -225,8 +234,11 @@
 bool DynTypedMatcher::matchesNoKindCheck(
 const ast_type_traits::DynTypedNode &DynNode, ASTMatchFinder *Finder,
 BoundNodesTreeBuilder *Builder) const {
-  assert(RestrictKind.isBaseOf(DynNode.getNodeKind()));
-  if (Implementation->dynMatches(DynNode, Finder, Builder)) {
+  auto N = Finder->getASTContext().TraverseIgnored(DynNode);
+  auto NodeKind = N.getNodeKind();
+
+  assert(RestrictKind.isBaseOf(NodeKind));
+  if (Implementation->dynMatches(N, Finder, Builder)) {
 return true;
   }
   // Delete all bindings when a matcher does not match.
Index: lib/ASTMatchers/ASTMatchFinder.cpp
===
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -59,10 +59,12 @@
   DynTypedMatcher::MatcherIDType MatcherID;
   ast_type_traits::DynTypedNode Node;
   BoundNodesTreeBuilder 

[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: include/clang/AST/ASTContext.h:562-563
 public:
+  ast_type_traits::TraversalKind GetTraversalKind() const { return Traversal; }
+  void SetTraversalKind(ast_type_traits::TraversalKind TK) { Traversal = TK; }
+

aaron.ballman wrote:
> This doesn't match our coding guidelines. Should be `getTraversalKind()`, 
> etc. Same below.
I think clang still uses uppercase names everywhere. Can you be more specific?



Comment at: include/clang/AST/ASTContext.h:565-568
+  const Expr *TraverseIgnored(const Expr *E);
+  Expr *TraverseIgnored(Expr *E);
+  ast_type_traits::DynTypedNode
+  TraverseIgnored(const ast_type_traits::DynTypedNode &N);

aaron.ballman wrote:
> Is there a reason we don't want these functions to be marked `const`?
It looks like they can be const.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:718
+template 
+internal::Matcher traverse(ast_type_traits::TraversalKind TK,
+  const internal::Matcher &InnerMatcher) {

aaron.ballman wrote:
> Is this an API we should be exposing to clang-query as well? Will those users 
> be able to use a string literal for the traversal kind, like they already do 
> for attribute kinds (etc)?
Yes, I thought about that, but in a follow-up patch. First, I aim to extend the 
`TraversalKind` enum with `TK_IgnoreInvisble`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61837



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

Re: r360637 - PR41817: Fix regression in r359260 that caused the MS compatibility

[Reid Kleckner via cfe-commits]

*From: *Richard Smith 
*Date: *Wed, May 15, 2019 at 5:43 PM

> On Wed, 15 May 2019 at 15:54, Reid Kleckner via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> We could probably re-land Richard's change (although it's quite a lot of
>> code...), but... drop the conflicting `static` on the floor
>> if hasLinkageBeenComputed() returns true. That would work around the
>> assert, right?
>>
>
> Yes, that seems like it could work, though I really don't like our
> generated code depending on when we happen to compute linkage. In the MIDL
> case, are there any uses of the global between the 'extern' declaration and
> the 'static' declaration?
>

Hm, there actually are uses, so I think my suggestion isn't enough. It
makes sense, MIDL is creating forward decls so they can be referenced
before they are defined.

This is what I looked at:
https://searchfox.org/mozilla-central/search?q=symbol:_Z34HandlerData__MIDL_TypeFormatString&redirect=false

Why were we getting static linkage before anyway? We just happened to look
at the last declaration instead of the first or canonical one when doing
IRGen?

In the context of C, downgrading from extern to static isn't such a big
deal. We could only implement it for C, since that's what MSVC does.

Given that this will require mutating IR that we've already created, I'd be
willing to look into it further.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61909: Add Clang shared library with C++ exports

[Wink Saville via Phabricator via cfe-commits]

winksaville added a comment.

Just to be clear, I have nothing to do with any distribution except as a user 
(Arch Linux) so please take
what I say and request with a huge grain of salt. As mentioned I have filed a 
bug 
against the clang package in Arch Linux so hopefully we'll be able to get them 
going in the right direction.

I do have another use case that I've done a little work on. For Pony 
 they have a problem tracking
the various versions of LLVM that distros use so I created a git submodule 
 which allows a ponyc
developer to work with multiple versions or variations of LLVM simultaneously 
and switch between them
relatively easily. Also, they will eventually switch to bundling libLLVM 
instead of using the system installed
version which could be any version. I have a standalone version of that at 
mkllvm-tool-chain .

Anyway, I'm obviously using "intall" style and I suggest having a 
`BUILD_CLANG_DYLIB` and `LINK_CLANG_DYLIB` would
be useful to more precisely control how a compiler might use the `libclang*`. 
Some users of LLVM might have a bunch of tools
that use various subparts of clang and they might be resident all the time. So 
startup performance isn't a problem, but
reducing the in memory foot print using shared libraries could be useful. Or 
not building the them at all if they are not
using them in anyway. So you're probably correct that for distros these flags 
shouldn't be used, but for other use cases
of LLVM I think there are.

In anycase, I think this is a worth while change as it and If you're still not 
convinced I like to see this in sooner rather than later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61909



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

[PATCH] D61835: Extract ASTDumper to a header file

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D61835#1505202 , @steveire wrote:

> In D61835#1505151 , @aaron.ballman 
> wrote:
>
> > In D61835#1505048 , @steveire 
> > wrote:
> >
> > > The users of the follow-up patch 
> > > https://reviews.llvm.org/D61837#change-x5mxz9Lpijjs need that 
> > > 'correctness', but also need the public API from `ASTNodeTraverser` on 
> > > the instance. (That patch also extends the public API for users).
> >
> >
> > I don't see anything in the follow-up patch that actually uses the 
> > `ASTDumper` class though, so I'm still a bit confused.
>
>
> Maybe my wording was confusing. I'll try again:
>
> 1. Anyone who wants traversal in the same way that dumping is done must use 
> an instance of the `ASTDumper` class. Is that much clear? Does my previous 
> response clarify why that is the case?


Yup, on the same page there for the most part. Is the "anyone" you refer to 
someone developing clang (or clang's tools), or third party? I've been 
envisioning clang development.

> 2. The follow-up patch https://reviews.llvm.org/D61837#change-x5mxz9Lpijjs 
> adds API to ASTNodeTraverser.

Yup, definitely can see that.

> 3. Anyone who wants traversal in the same way that dumping is done and who 
> needs to call API on the instance which is provided by ASTNodeTraverser 
> (which ASTDumper inherits) needs to use ASTDumper. For example my UI. See my 
> EuroLLVM talk for more: 
> https://steveire.wordpress.com/2019/04/30/the-future-of-ast-matching-refactoring-tools-eurollvm-and-accu/

Do they? Why is the `ASTNodeTraverser` insufficient? e.g., can they be given a 
reference to the base class rather than the derived class? For instance, 
`Decl::dump()` creates an `ASTDumper` instance, but to initiate the traversal, 
it calls `Visit()` from the base class.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61835



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

[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/AST/ASTContext.h:562-563
 public:
+  ast_type_traits::TraversalKind GetTraversalKind() const { return Traversal; }
+  void SetTraversalKind(ast_type_traits::TraversalKind TK) { Traversal = TK; }
+

steveire wrote:
> aaron.ballman wrote:
> > This doesn't match our coding guidelines. Should be `getTraversalKind()`, 
> > etc. Same below.
> I think clang still uses uppercase names everywhere. Can you be more specific?
No, Clang doesn't use uppercase names everywhere -- we're consistently 
inconsistent and it depends mostly on the age of when the code was introduced 
and what the surrounding code looks like. We still follow the usual coding 
style guidelines for naming conventions -- stick with the convention used by 
nearby code if it's already consistent, otherwise follow the coding style rules.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:716
+/// \endcode
+/// matches the return statement with "ret" bound to "a".
+template 

aaron.ballman wrote:
> Copy pasta?
You dropped the interesting bit from the documentation here -- you should add 
in what the matcher is matching (which makes the preceding "The matcher \code 
... \endcode" grammatical again).



Comment at: include/clang/ASTMatchers/ASTMatchers.h:718
+template 
+internal::Matcher traverse(ast_type_traits::TraversalKind TK,
+  const internal::Matcher &InnerMatcher) {

steveire wrote:
> aaron.ballman wrote:
> > Is this an API we should be exposing to clang-query as well? Will those 
> > users be able to use a string literal for the traversal kind, like they 
> > already do for attribute kinds (etc)?
> Yes, I thought about that, but in a follow-up patch. First, I aim to extend 
> the `TraversalKind` enum with `TK_IgnoreInvisble`.
This is new functionality, so why do you want to wait for a follow-up patch (is 
it somehow more involved)? We typically add support for dynamic matchers at the 
same time we add support for the static matchers because otherwise the two get 
frustratingly out of sync.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61837



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

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

[Jian Cai via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp:9
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer pipe2() to pipe() because 
pipe2() allows O_CLOEXEC [android-cloexec-pipe]
+  // CHECK-FIXES: pipe2(pipefd, O_CLOEXEC);
+}

george.burgess.iv wrote:
> (Do we have a CHECK-FIXES-NOT or CHECK-MESSAGES-NOT to apply to the things 
> below? Or are the CHECKs here meant to be complete like clang's `-verify`?)
Based on Testing Checks 
https://clang.llvm.org/extra/clang-tidy/Contributing.html, CHECK-MASSAGES and 
CHECK-FIXES are sufficient for clang-tidy checks. I have double checked the 
tests for similar checks and they don't seem to have additional FileCheck 
invocations other than these two.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61967



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