[PATCH] D93031: Enable fexec-charset option

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3583-3584
 
+def fexec_charset : Separate<["-"], "fexec-charset">, 
MetaVarName<"">,
+  HelpText<"Set the execution  for string and character literals">;
 def target_cpu : Separate<["-"], "target-cpu">,

How about substituting "character set", "character encoding", or "charset" for 
"codepage"?

This doesn't state what names are recognized.  The ones provided by the system 
iconv() implementation (as is the case for gcc)?  Or all names and aliases 
specified by the [[ 
https://www.iana.org/assignments/character-sets/character-sets.xhtml | IANA 
character set registry ]]?

The set of recognized names can be a superset of the names that are actually 
supported.



Comment at: clang/include/clang/Lex/LiteralSupport.h:192
+ConversionState translationState = TranslateToExecCharset);
+  ConversionState TranslationState;
 

Does the conversion state need to be persisted as a data member?  The literal 
is consumed in the constructor.



Comment at: clang/include/clang/Lex/LiteralSupport.h:244
   bool Pascal;
+  ConversionState TranslationState;
+

Same concern here with respect to persisting the conversion state as a data 
member.



Comment at: clang/include/clang/Lex/LiteralSupport.h:246
+
+  static LiteralTranslator Translator;
 

This static data member will presumably need to be lifted to per-instance state 
as Richard mentioned elsewhere.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5969-5977
+  // Pass all -fexec-charset options to cc1.
+  std::vector vList =
+  Args.getAllArgValues(options::OPT_fexec_charset_EQ);
+  // Set the default fexec-charset as the system charset.
+  CmdArgs.push_back("-fexec-charset");
+  CmdArgs.push_back(Args.MakeArgString(Triple.getSystemCharset()));
+  for (auto it = vList.begin(), ie = vList.end(); it != ie; ++it) {

I think it would be preferable to diagnose an unrecognized character encoding 
name here if possible.  The current changes will result in an unrecognized name 
(as opposed to one that is unsupported for the target) being diagnosed for each 
compiler instance.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3573
+StringRef Value = ExecCharset->getValue();
+Opts.ExecCharset = (std::string)Value;
+  }

I wouldn't expect the cast to `std::string` to be needed here.



Comment at: clang/lib/Lex/LiteralSupport.cpp:231-239
+  if (Translate && Converter) {
+// ResultChar is either UTF-8 or ASCII literal and can only be converted
+// to EBCDIC on z/OS if the character can be represented in one byte.
+if (ResultChar < 0x100) {
+  SmallString<8> ResultCharConv;
+  Converter->convert(StringRef((char *)), ResultCharConv);
+  void *Pointer = 

rsmith wrote:
> Is it correct, in general, to do character-at-a-time translation here, when 
> processing a string literal? I would expect there to be some (stateful) 
> target character sets where that's not correct.
For stateful encodings, I can imagine that state would have to be transitioned 
to the initial state before translating the escape sequence.  I suspect support 
for stateful encodings is not a goal at this time.



Comment at: clang/lib/Lex/LiteralSupport.cpp:234
+// to EBCDIC on z/OS if the character can be represented in one byte.
+if (ResultChar < 0x100) {
+  SmallString<8> ResultCharConv;

What should happen if `ResultChar` >= 0x100?  IBM-1047 does have representation 
for other UTF-8 characters.  Regardless, it seems `ResultChar` should be 
converted to something.



Comment at: clang/lib/Lex/LiteralSupport.cpp:1742-1745
   EncodeUCNEscape(ThisTokBegin, ThisTokBuf, ThisTokEnd,
   ResultPtr, hadError,
   FullSourceLoc(StringToks[i].getLocation(), SM),
   CharByteWidth, Diags, Features);

UCNs will require conversion here.



Comment at: llvm/lib/Support/Triple.cpp:1028-1029
+StringRef Triple::getSystemCharset() const {
+  if (getOS() == llvm::Triple::ZOS)
+return "IBM-1047";
+  return "UTF-8";

No support for targeting the z/OS Enhanced ASCII run-time?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93031

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Douglas Chen via Phabricator via cfe-commits]

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

Hi @njames93, thank you for your review suggestions, I have improved them and 
against my change to the main branch.

I encounter a problem on the Buildbot for Windows only. Several people 
encounter also to the same problem that their build failed at an unrelated 
regression test (llvm\test\CodeGen\XCore\threads.ll).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

[PATCH] D92897: Set legacy pass manager OptBisect to same as NPM OptBisect

[Samuel Eubanks via Phabricator via cfe-commits]

swamulism updated this revision to Diff 311360.
swamulism added a comment.

Update comment strings


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92897

Files:
  clang/test/CodeGen/new-pass-manager-opt-bisect.c
  llvm/include/llvm/IR/OptBisect.h
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/IR/LLVMContextImpl.cpp
  llvm/lib/IR/OptBisect.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp

Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -624,11 +624,11 @@
 
 void OptBisectInstrumentation::registerCallbacks(
 PassInstrumentationCallbacks ) {
-  if (!isEnabled())
+  if (!OptBisector->isEnabled())
 return;
-
-  PIC.registerShouldRunOptionalPassCallback([this](StringRef PassID, Any IR) {
-return isIgnored(PassID) || checkPass(PassID, getBisectDescription(IR));
+  PIC.registerShouldRunOptionalPassCallback([](StringRef PassID, Any IR) {
+return isIgnored(PassID) ||
+   OptBisector->checkPass(PassID, getBisectDescription(IR));
   });
 }
 
Index: llvm/lib/IR/OptBisect.cpp
===
--- llvm/lib/IR/OptBisect.cpp
+++ llvm/lib/IR/OptBisect.cpp
@@ -54,3 +54,5 @@
   printPassMessage(PassName, CurBisectNum, TargetDesc, ShouldRun);
   return ShouldRun;
 }
+
+ManagedStatic llvm::OptBisector;
Index: llvm/lib/IR/LLVMContextImpl.cpp
===
--- llvm/lib/IR/LLVMContextImpl.cpp
+++ llvm/lib/IR/LLVMContextImpl.cpp
@@ -219,19 +219,8 @@
 SSNs[SSE.second] = SSE.first();
 }
 
-/// Singleton instance of the OptBisect class.
-///
-/// This singleton is accessed via the LLVMContext::getOptPassGate() function.
-/// It provides a mechanism to disable passes and individual optimizations at
-/// compile time based on a command line option (-opt-bisect-limit) in order to
-/// perform a bisecting search for optimization-related problems.
-///
-/// Even if multiple LLVMContext objects are created, they will all return the
-/// same instance of OptBisect in order to provide a single bisect count.  Any
-/// code that uses the OptBisect object should be serialized when bisection is
-/// enabled in order to enable a consistent bisect count.
-static ManagedStatic OptBisector;
-
+/// Gets the OptPassGate for this LLVMContextImpl, which defaults to the
+/// singleton OptBisect if not explicitly set.
 OptPassGate ::getOptPassGate() const {
   if (!OPG)
 OPG = &(*OptBisector);
Index: llvm/include/llvm/Passes/StandardInstrumentations.h
===
--- llvm/include/llvm/Passes/StandardInstrumentations.h
+++ llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -72,7 +72,7 @@
   bool shouldRun(StringRef PassID, Any IR);
 };
 
-class OptBisectInstrumentation : public OptBisect {
+class OptBisectInstrumentation {
 public:
   OptBisectInstrumentation() {}
   void registerCallbacks(PassInstrumentationCallbacks );
Index: llvm/include/llvm/IR/OptBisect.h
===
--- llvm/include/llvm/IR/OptBisect.h
+++ llvm/include/llvm/IR/OptBisect.h
@@ -15,6 +15,7 @@
 #define LLVM_IR_OPTBISECT_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ManagedStatic.h"
 
 namespace llvm {
 
@@ -32,7 +33,7 @@
 return true;
   }
 
-  /// isEnabled should return true before calling shouldRunPass
+  /// isEnabled() should return true before calling shouldRunPass().
   virtual bool isEnabled() const { return false; }
 };
 
@@ -53,6 +54,14 @@
 
   virtual ~OptBisect() = default;
 
+  /// Checks the bisect limit to determine if the specified pass should run.
+  ///
+  /// This forwards to checkPass().
+  bool shouldRunPass(const Pass *P, StringRef IRDescription) override;
+
+  /// isEnabled() should return true before calling shouldRunPass().
+  bool isEnabled() const override { return BisectEnabled; }
+
   /// Checks the bisect limit to determine if the specified pass should run.
   ///
   /// If the bisect limit is set to -1, the function prints a message describing
@@ -64,12 +73,6 @@
   /// Most passes should not call this routine directly. Instead, they are
   /// called through helper routines provided by the pass base classes.  For
   /// instance, function passes should call FunctionPass::skipFunction().
-  bool shouldRunPass(const Pass *P, StringRef IRDescription) override;
-
-  /// isEnabled should return true before calling shouldRunPass
-  bool isEnabled() const override { return BisectEnabled; }
-
-protected:
   bool checkPass(const StringRef PassName, const StringRef TargetDesc);
 
 private:
@@ -77,6 +80,9 @@
   unsigned LastBisectNum = 0;
 };
 
+/// Singleton instance of the OptBisect class, so multiple pass managers don't
+/// 

[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

[Mircea Trofin via Phabricator via cfe-commits]

mtrofin updated this revision to Diff 311359.
mtrofin marked an inline comment as done.
mtrofin added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

added check for when a prefix has conflicts for all functions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93078

Files:
  clang/test/utils/update_cc_test_checks/Inputs/prefix-never-matches.cpp
  clang/test/utils/update_cc_test_checks/prefix-never-matches.test
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/common-label-different-bodies-1.ll
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/common-label-different-bodies-2.ll
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/common-label-different-bodies-3.ll
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/prefix-never-matches.ll
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/common-label-different-bodies.test
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/prefix-never-matches.test
  
llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/prefix-never-matches.ll
  llvm/test/tools/UpdateTestChecks/update_test_checks/prefix-never-matches.test
  llvm/utils/UpdateTestChecks/common.py
  llvm/utils/update_analyze_test_checks.py
  llvm/utils/update_cc_test_checks.py
  llvm/utils/update_llc_test_checks.py
  llvm/utils/update_test_checks.py

Index: llvm/utils/update_test_checks.py
===
--- llvm/utils/update_test_checks.py
+++ llvm/utils/update_test_checks.py
@@ -117,7 +117,8 @@
   common.OPT_FUNCTION_RE, common.scrub_body, [],
   raw_tool_output, prefixes, func_dict, func_order, ti.args.verbose,
   ti.args.function_signature, ti.args.check_attributes)
-
+
+common.warn_on_failed_prefixes(func_dict)
 is_in_function = False
 is_in_function_start = False
 prefix_set = set([prefix for prefixes, _ in prefix_list for prefix in prefixes])
Index: llvm/utils/update_llc_test_checks.py
===
--- llvm/utils/update_llc_test_checks.py
+++ llvm/utils/update_llc_test_checks.py
@@ -118,6 +118,8 @@
   asm.build_function_body_dictionary_for_triple(ti.args, raw_tool_output,
   triple, prefixes, func_dict, func_order)
 
+common.warn_on_failed_prefixes(func_dict)
+
 is_in_function = False
 is_in_function_start = False
 func_name = None
Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -266,7 +266,8 @@
   # mangled names. Forward all clang args for now.
   for k, v in get_line2spell_and_mangled(ti.args, clang_args).items():
 line2spell_and_mangled_list[k].append(v)
-
+
+common.warn_on_failed_prefixes(func_dict)
 global_vars_seen_dict = {}
 prefix_set = set([prefix for p in run_list for prefix in p[0]])
 output_lines = []
Index: llvm/utils/update_analyze_test_checks.py
===
--- llvm/utils/update_analyze_test_checks.py
+++ llvm/utils/update_analyze_test_checks.py
@@ -125,7 +125,8 @@
 common.build_function_body_dictionary(
   common.ANALYZE_FUNCTION_RE, common.scrub_body, [],
   raw_tool_output, prefixes, func_dict, func_order, args.verbose, False, False)
-
+
+common.warn_on_failed_prefixes(func_dict)
 is_in_function = False
 is_in_function_start = False
 prefix_set = set([prefix for prefixes, _ in prefix_list for prefix in prefixes])
Index: llvm/utils/UpdateTestChecks/common.py
===
--- llvm/utils/UpdateTestChecks/common.py
+++ llvm/utils/UpdateTestChecks/common.py
@@ -258,6 +258,20 @@
   def __str__(self):
 return self.scrub
 
+def get_failed_prefixes(func_dict):
+  # This returns the list of those prefixes that failed to match any function,
+  # because there were conflicting bodies produced by different RUN lines, in
+  # all instances of the prefix. Effectivelly, this prefix is unused and should
+  # be removed.
+  for prefix in func_dict:
+if (not [fct for fct in func_dict[prefix] 
+ if func_dict[prefix][fct] is not None]):
+  yield prefix
+
+def warn_on_failed_prefixes(func_dict):
+  for prefix in get_failed_prefixes(func_dict):
+  warn('Prefix %s had conflicting output from different RUN lines for all functions' % (prefix,))
+
 def build_function_body_dictionary(function_re, scrubber, scrubber_args, raw_tool_output, prefixes, func_dict, func_order, verbose, record_args, check_attributes):
   for m in function_re.finditer(raw_tool_output):
 if not m:
@@ -287,17 +301,24 @@
 print('  ' + l, file=sys.stderr)
 for prefix in prefixes:
  

[PATCH] D92897: Set legacy pass manager OptBisect to same as NPM OptBisect

[Samuel Eubanks via Phabricator via cfe-commits]

swamulism updated this revision to Diff 311358.
swamulism marked 5 inline comments as done.
swamulism added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92897

Files:
  clang/test/CodeGen/new-pass-manager-opt-bisect.c
  llvm/include/llvm/IR/OptBisect.h
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/IR/LLVMContextImpl.cpp
  llvm/lib/IR/OptBisect.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp

Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -624,11 +624,11 @@
 
 void OptBisectInstrumentation::registerCallbacks(
 PassInstrumentationCallbacks ) {
-  if (!isEnabled())
+  if (!OptBisector->isEnabled())
 return;
-
-  PIC.registerShouldRunOptionalPassCallback([this](StringRef PassID, Any IR) {
-return isIgnored(PassID) || checkPass(PassID, getBisectDescription(IR));
+  PIC.registerShouldRunOptionalPassCallback([](StringRef PassID, Any IR) {
+return isIgnored(PassID) ||
+   OptBisector->checkPass(PassID, getBisectDescription(IR));
   });
 }
 
Index: llvm/lib/IR/OptBisect.cpp
===
--- llvm/lib/IR/OptBisect.cpp
+++ llvm/lib/IR/OptBisect.cpp
@@ -54,3 +54,5 @@
   printPassMessage(PassName, CurBisectNum, TargetDesc, ShouldRun);
   return ShouldRun;
 }
+
+ManagedStatic llvm::OptBisector;
Index: llvm/lib/IR/LLVMContextImpl.cpp
===
--- llvm/lib/IR/LLVMContextImpl.cpp
+++ llvm/lib/IR/LLVMContextImpl.cpp
@@ -219,19 +219,8 @@
 SSNs[SSE.second] = SSE.first();
 }
 
-/// Singleton instance of the OptBisect class.
-///
-/// This singleton is accessed via the LLVMContext::getOptPassGate() function.
-/// It provides a mechanism to disable passes and individual optimizations at
-/// compile time based on a command line option (-opt-bisect-limit) in order to
-/// perform a bisecting search for optimization-related problems.
-///
-/// Even if multiple LLVMContext objects are created, they will all return the
-/// same instance of OptBisect in order to provide a single bisect count.  Any
-/// code that uses the OptBisect object should be serialized when bisection is
-/// enabled in order to enable a consistent bisect count.
-static ManagedStatic OptBisector;
-
+/// Gets the OptPassGate for this LLVMContextImpl, which defaults to the
+/// singleton OptBisect if not explicitly set.
 OptPassGate ::getOptPassGate() const {
   if (!OPG)
 OPG = &(*OptBisector);
Index: llvm/include/llvm/Passes/StandardInstrumentations.h
===
--- llvm/include/llvm/Passes/StandardInstrumentations.h
+++ llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -72,7 +72,7 @@
   bool shouldRun(StringRef PassID, Any IR);
 };
 
-class OptBisectInstrumentation : public OptBisect {
+class OptBisectInstrumentation {
 public:
   OptBisectInstrumentation() {}
   void registerCallbacks(PassInstrumentationCallbacks );
Index: llvm/include/llvm/IR/OptBisect.h
===
--- llvm/include/llvm/IR/OptBisect.h
+++ llvm/include/llvm/IR/OptBisect.h
@@ -15,6 +15,7 @@
 #define LLVM_IR_OPTBISECT_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ManagedStatic.h"
 
 namespace llvm {
 
@@ -53,6 +54,14 @@
 
   virtual ~OptBisect() = default;
 
+  /// Checks the bisect limit to determine if the specified pass should run.
+  ///
+  /// This forwards to checkPass().
+  bool shouldRunPass(const Pass *P, StringRef IRDescription) override;
+
+  /// isEnabled should return true before calling shouldRunPass
+  bool isEnabled() const override { return BisectEnabled; }
+
   /// Checks the bisect limit to determine if the specified pass should run.
   ///
   /// If the bisect limit is set to -1, the function prints a message describing
@@ -64,12 +73,6 @@
   /// Most passes should not call this routine directly. Instead, they are
   /// called through helper routines provided by the pass base classes.  For
   /// instance, function passes should call FunctionPass::skipFunction().
-  bool shouldRunPass(const Pass *P, StringRef IRDescription) override;
-
-  /// isEnabled should return true before calling shouldRunPass
-  bool isEnabled() const override { return BisectEnabled; }
-
-protected:
   bool checkPass(const StringRef PassName, const StringRef TargetDesc);
 
 private:
@@ -77,6 +80,9 @@
   unsigned LastBisectNum = 0;
 };
 
+/// Singleton instance of the OptBisect class, so multiple pass managers don't
+/// need to coordinate their uses of OptBisect.
+extern ManagedStatic OptBisector;
 } // end namespace llvm
 
 #endif // LLVM_IR_OPTBISECT_H
Index: clang/test/CodeGen/new-pass-manager-opt-bisect.c

[PATCH] D92897: Set legacy pass manager OptBisect to same as NPM OptBisect

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added inline comments.



Comment at: llvm/include/llvm/IR/OptBisect.h:58
+  /// Checks the bisect limit to determine if the specified pass should run.
+  /// This calls checkPass
+  bool shouldRunPass(const Pass *P, StringRef IRDescription) override;





Comment at: llvm/include/llvm/IR/OptBisect.h:82
 
+/// Singleton instance of the OptBisect class.
+extern ManagedStatic OptBisector;

should probably mention that this is so that multiple pass managers don't need 
to coordinate their uses of OptBisect



Comment at: llvm/lib/IR/LLVMContextImpl.cpp:222
 
-/// Singleton instance of the OptBisect class.
-///
-/// This singleton is accessed via the LLVMContext::getOptPassGate() function.
-/// It provides a mechanism to disable passes and individual optimizations at
-/// compile time based on a command line option (-opt-bisect-limit) in order to
-/// perform a bisecting search for optimization-related problems.
-///
-/// Even if multiple LLVMContext objects are created, they will all return the
-/// same instance of OptBisect in order to provide a single bisect count.  Any
-/// code that uses the OptBisect object should be serialized when bisection is
-/// enabled in order to enable a consistent bisect count.
-static ManagedStatic OptBisector;
-
+/// Gets the singleton instance OptBisect if not already set
 OptPassGate ::getOptPassGate() const {





Comment at: llvm/lib/IR/OptBisect.cpp:58
+
+ManagedStatic llvm::OptBisector;

newline at end of file?



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:28
 #include "llvm/IR/Verifier.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"

is this include necessary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92897

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

[PATCH] D92897: Set legacy pass manager OptBisect to same as NPM OptBisect

[Samuel Eubanks via Phabricator via cfe-commits]

swamulism added a comment.

Thank you for the comments and code review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92897

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

[PATCH] D92897: Set legacy pass manager OptBisect to same as NPM OptBisect

[Samuel Eubanks via Phabricator via cfe-commits]

swamulism updated this revision to Diff 311357.
swamulism added a comment.
Herald added subscribers: dexonsmith, hiraditya.

Updating to set npm OptBisect to use global singleton OptBisect instead


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92897

Files:
  clang/test/CodeGen/new-pass-manager-opt-bisect.c
  llvm/include/llvm/IR/OptBisect.h
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/IR/LLVMContextImpl.cpp
  llvm/lib/IR/OptBisect.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp

Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -21,9 +21,11 @@
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/OptBisect.h"
 #include "llvm/IR/PassInstrumentation.h"
 #include "llvm/IR/PrintPasses.h"
 #include "llvm/IR/Verifier.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -624,11 +626,11 @@
 
 void OptBisectInstrumentation::registerCallbacks(
 PassInstrumentationCallbacks ) {
-  if (!isEnabled())
+  if (!OptBisector->isEnabled())
 return;
-
-  PIC.registerShouldRunOptionalPassCallback([this](StringRef PassID, Any IR) {
-return isIgnored(PassID) || checkPass(PassID, getBisectDescription(IR));
+  PIC.registerShouldRunOptionalPassCallback([](StringRef PassID, Any IR) {
+return isIgnored(PassID) ||
+   OptBisector->checkPass(PassID, getBisectDescription(IR));
   });
 }
 
Index: llvm/lib/IR/OptBisect.cpp
===
--- llvm/lib/IR/OptBisect.cpp
+++ llvm/lib/IR/OptBisect.cpp
@@ -54,3 +54,5 @@
   printPassMessage(PassName, CurBisectNum, TargetDesc, ShouldRun);
   return ShouldRun;
 }
+
+ManagedStatic llvm::OptBisector;
\ No newline at end of file
Index: llvm/lib/IR/LLVMContextImpl.cpp
===
--- llvm/lib/IR/LLVMContextImpl.cpp
+++ llvm/lib/IR/LLVMContextImpl.cpp
@@ -219,19 +219,7 @@
 SSNs[SSE.second] = SSE.first();
 }
 
-/// Singleton instance of the OptBisect class.
-///
-/// This singleton is accessed via the LLVMContext::getOptPassGate() function.
-/// It provides a mechanism to disable passes and individual optimizations at
-/// compile time based on a command line option (-opt-bisect-limit) in order to
-/// perform a bisecting search for optimization-related problems.
-///
-/// Even if multiple LLVMContext objects are created, they will all return the
-/// same instance of OptBisect in order to provide a single bisect count.  Any
-/// code that uses the OptBisect object should be serialized when bisection is
-/// enabled in order to enable a consistent bisect count.
-static ManagedStatic OptBisector;
-
+/// Gets the singleton instance OptBisect if not already set
 OptPassGate ::getOptPassGate() const {
   if (!OPG)
 OPG = &(*OptBisector);
Index: llvm/include/llvm/Passes/StandardInstrumentations.h
===
--- llvm/include/llvm/Passes/StandardInstrumentations.h
+++ llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -72,7 +72,7 @@
   bool shouldRun(StringRef PassID, Any IR);
 };
 
-class OptBisectInstrumentation : public OptBisect {
+class OptBisectInstrumentation {
 public:
   OptBisectInstrumentation() {}
   void registerCallbacks(PassInstrumentationCallbacks );
Index: llvm/include/llvm/IR/OptBisect.h
===
--- llvm/include/llvm/IR/OptBisect.h
+++ llvm/include/llvm/IR/OptBisect.h
@@ -15,6 +15,7 @@
 #define LLVM_IR_OPTBISECT_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ManagedStatic.h"
 
 namespace llvm {
 
@@ -53,6 +54,13 @@
 
   virtual ~OptBisect() = default;
 
+  /// Checks the bisect limit to determine if the specified pass should run.
+  /// This calls checkPass
+  bool shouldRunPass(const Pass *P, StringRef IRDescription) override;
+
+  /// isEnabled should return true before calling shouldRunPass
+  bool isEnabled() const override { return BisectEnabled; }
+
   /// Checks the bisect limit to determine if the specified pass should run.
   ///
   /// If the bisect limit is set to -1, the function prints a message describing
@@ -64,12 +72,6 @@
   /// Most passes should not call this routine directly. Instead, they are
   /// called through helper routines provided by the pass base classes.  For
   /// instance, function passes should call FunctionPass::skipFunction().
-  bool shouldRunPass(const Pass *P, StringRef IRDescription) override;
-
-  /// isEnabled should return true before calling shouldRunPass
-  bool isEnabled() const override { return BisectEnabled; }
-
-protected:
   bool 

[PATCH] D92971: clang-import-test: Clean up error output for files that cannot be found

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks for the quick fix! The bot is happy again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92971

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

[PATCH] D92971: clang-import-test: Clean up error output for files that cannot be found

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D92971#2449934 , @dexonsmith wrote:

> In D92971#2449932 , @thakis wrote:
>
>> This breaks tests on Windows: http://45.33.8.238/win/29686/step_7.txt
>>
>> PTAL, and please revert for now if it takes a while to fix.
>
> I'll fix now.

e095959e0c23e250d6ad1dbe3612291736d12e1a 
 makes it 
not platform-dependent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92971

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

[clang] e095959 - Fixup for 8c86197de3cba4257f26133e837d64e5f8ece210 to avoid making it platform-dependent

[Duncan P. N. Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2020-12-11T17:34:00-08:00
New Revision: e095959e0c23e250d6ad1dbe3612291736d12e1a

URL: 
https://github.com/llvm/llvm-project/commit/e095959e0c23e250d6ad1dbe3612291736d12e1a
DIFF: 
https://github.com/llvm/llvm-project/commit/e095959e0c23e250d6ad1dbe3612291736d12e1a.diff

LOG: Fixup for 8c86197de3cba4257f26133e837d64e5f8ece210 to avoid making it 
platform-dependent

Added: 


Modified: 
clang/tools/clang-import-test/clang-import-test.cpp

Removed: 




diff  --git a/clang/tools/clang-import-test/clang-import-test.cpp 
b/clang/tools/clang-import-test/clang-import-test.cpp
index 5e84c5f97851..df173cf49f35 100644
--- a/clang/tools/clang-import-test/clang-import-test.cpp
+++ b/clang/tools/clang-import-test/clang-import-test.cpp
@@ -291,9 +291,9 @@ llvm::Error ParseSource(const std::string , 
CompilerInstance ,
   SourceManager  = CI.getSourceManager();
   auto FE = CI.getFileManager().getFileRef(Path);
   if (!FE) {
+llvm::consumeError(FE.takeError());
 return llvm::make_error(
-llvm::Twine(llvm::toString(FE.takeError())) + ": " + Path,
-std::error_code());
+llvm::Twine("No such file or directory: ", Path), std::error_code());
   }
   SM.setMainFileID(SM.createFileID(*FE, SourceLocation(), SrcMgr::C_User));
   ParseAST(CI.getPreprocessor(), , CI.getASTContext());



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

[PATCH] D92971: clang-import-test: Clean up error output for files that cannot be found

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D92971#2449932 , @thakis wrote:

> This breaks tests on Windows: http://45.33.8.238/win/29686/step_7.txt
>
> PTAL, and please revert for now if it takes a while to fix.

I'll fix now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92971

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

[PATCH] D92971: clang-import-test: Clean up error output for files that cannot be found

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This breaks tests on Windows: http://45.33.8.238/win/29686/step_7.txt

PTAL, and please revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92971

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

[PATCH] D92971: clang-import-test: Clean up error output for files that cannot be found

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c86197de3cb: clang-import-test: Clean up error output for 
files that cannot be found (authored by dexonsmith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92971

Files:
  clang/test/Import/missing-import/test.c
  clang/tools/clang-import-test/clang-import-test.cpp


Index: clang/tools/clang-import-test/clang-import-test.cpp
===
--- clang/tools/clang-import-test/clang-import-test.cpp
+++ clang/tools/clang-import-test/clang-import-test.cpp
@@ -289,10 +289,11 @@
 llvm::Error ParseSource(const std::string , CompilerInstance ,
 ASTConsumer ) {
   SourceManager  = CI.getSourceManager();
-  auto FE = CI.getFileManager().getFile(Path);
+  auto FE = CI.getFileManager().getFileRef(Path);
   if (!FE) {
 return llvm::make_error(
-llvm::Twine("Couldn't open ", Path), std::error_code());
+llvm::Twine(llvm::toString(FE.takeError())) + ": " + Path,
+std::error_code());
   }
   SM.setMainFileID(SM.createFileID(*FE, SourceLocation(), SrcMgr::C_User));
   ParseAST(CI.getPreprocessor(), , CI.getASTContext());
@@ -360,7 +361,7 @@
   for (auto I : Imports) {
 llvm::Expected ImportCI = Parse(I, {}, false, false);
 if (auto E = ImportCI.takeError()) {
-  llvm::errs() << llvm::toString(std::move(E));
+  llvm::errs() << "error: " << llvm::toString(std::move(E)) << "\n";
   exit(-1);
 }
 ImportCIs.push_back(std::move(*ImportCI));
@@ -379,7 +380,7 @@
   Parse(Expression, (Direct && !UseOrigins) ? ImportCIs : IndirectCIs,
 DumpAST, DumpIR);
   if (auto E = ExpressionCI.takeError()) {
-llvm::errs() << llvm::toString(std::move(E));
+llvm::errs() << "error: " << llvm::toString(std::move(E)) << "\n";
 exit(-1);
   }
   Forget(*ExpressionCI, (Direct && !UseOrigins) ? ImportCIs : IndirectCIs);
Index: clang/test/Import/missing-import/test.c
===
--- clang/test/Import/missing-import/test.c
+++ clang/test/Import/missing-import/test.c
@@ -1,5 +1,5 @@
 // RUN: not clang-import-test -import %S/Inputs/S.c -expression %s 2>&1 | 
FileCheck %s
-// CHECK: {{.*}}Couldn't open{{.*}}Inputs/S.c{{.*}}
+// CHECK: error: No such file or directory: {{.*}}Inputs/S.c{{$}}
 void expr() {
   struct S MyS;
   void *MyPtr = 


Index: clang/tools/clang-import-test/clang-import-test.cpp
===
--- clang/tools/clang-import-test/clang-import-test.cpp
+++ clang/tools/clang-import-test/clang-import-test.cpp
@@ -289,10 +289,11 @@
 llvm::Error ParseSource(const std::string , CompilerInstance ,
 ASTConsumer ) {
   SourceManager  = CI.getSourceManager();
-  auto FE = CI.getFileManager().getFile(Path);
+  auto FE = CI.getFileManager().getFileRef(Path);
   if (!FE) {
 return llvm::make_error(
-llvm::Twine("Couldn't open ", Path), std::error_code());
+llvm::Twine(llvm::toString(FE.takeError())) + ": " + Path,
+std::error_code());
   }
   SM.setMainFileID(SM.createFileID(*FE, SourceLocation(), SrcMgr::C_User));
   ParseAST(CI.getPreprocessor(), , CI.getASTContext());
@@ -360,7 +361,7 @@
   for (auto I : Imports) {
 llvm::Expected ImportCI = Parse(I, {}, false, false);
 if (auto E = ImportCI.takeError()) {
-  llvm::errs() << llvm::toString(std::move(E));
+  llvm::errs() << "error: " << llvm::toString(std::move(E)) << "\n";
   exit(-1);
 }
 ImportCIs.push_back(std::move(*ImportCI));
@@ -379,7 +380,7 @@
   Parse(Expression, (Direct && !UseOrigins) ? ImportCIs : IndirectCIs,
 DumpAST, DumpIR);
   if (auto E = ExpressionCI.takeError()) {
-llvm::errs() << llvm::toString(std::move(E));
+llvm::errs() << "error: " << llvm::toString(std::move(E)) << "\n";
 exit(-1);
   }
   Forget(*ExpressionCI, (Direct && !UseOrigins) ? ImportCIs : IndirectCIs);
Index: clang/test/Import/missing-import/test.c
===
--- clang/test/Import/missing-import/test.c
+++ clang/test/Import/missing-import/test.c
@@ -1,5 +1,5 @@
 // RUN: not clang-import-test -import %S/Inputs/S.c -expression %s 2>&1 | FileCheck %s
-// CHECK: {{.*}}Couldn't open{{.*}}Inputs/S.c{{.*}}
+// CHECK: error: No such file or directory: {{.*}}Inputs/S.c{{$}}
 void expr() {
   struct S MyS;
   void *MyPtr = 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 8c86197 - clang-import-test: Clean up error output for files that cannot be found

[Duncan P. N. Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2020-12-11T17:07:58-08:00
New Revision: 8c86197de3cba4257f26133e837d64e5f8ece210

URL: 
https://github.com/llvm/llvm-project/commit/8c86197de3cba4257f26133e837d64e5f8ece210
DIFF: 
https://github.com/llvm/llvm-project/commit/8c86197de3cba4257f26133e837d64e5f8ece210.diff

LOG: clang-import-test: Clean up error output for files that cannot be found

Pass on the filesystem error string `FileManager::getFileRef` in
`clang-import-test`'s `ParseSource` function. Also include "error:" and
a newline in the output. As a side effect, migrate to the `FileEntryRef`
overload of `SourceManager::createFileID`.

No real functionality change here, just slightly better output on error.

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

Added: 


Modified: 
clang/test/Import/missing-import/test.c
clang/tools/clang-import-test/clang-import-test.cpp

Removed: 




diff  --git a/clang/test/Import/missing-import/test.c 
b/clang/test/Import/missing-import/test.c
index acf6389cc5fc..9a16c2bf4ac8 100644
--- a/clang/test/Import/missing-import/test.c
+++ b/clang/test/Import/missing-import/test.c
@@ -1,5 +1,5 @@
 // RUN: not clang-import-test -import %S/Inputs/S.c -expression %s 2>&1 | 
FileCheck %s
-// CHECK: {{.*}}Couldn't open{{.*}}Inputs/S.c{{.*}}
+// CHECK: error: No such file or directory: {{.*}}Inputs/S.c{{$}}
 void expr() {
   struct S MyS;
   void *MyPtr = 

diff  --git a/clang/tools/clang-import-test/clang-import-test.cpp 
b/clang/tools/clang-import-test/clang-import-test.cpp
index eca3012957a3..5e84c5f97851 100644
--- a/clang/tools/clang-import-test/clang-import-test.cpp
+++ b/clang/tools/clang-import-test/clang-import-test.cpp
@@ -289,10 +289,11 @@ CIAndOrigins BuildIndirect(CIAndOrigins ) {
 llvm::Error ParseSource(const std::string , CompilerInstance ,
 ASTConsumer ) {
   SourceManager  = CI.getSourceManager();
-  auto FE = CI.getFileManager().getFile(Path);
+  auto FE = CI.getFileManager().getFileRef(Path);
   if (!FE) {
 return llvm::make_error(
-llvm::Twine("Couldn't open ", Path), std::error_code());
+llvm::Twine(llvm::toString(FE.takeError())) + ": " + Path,
+std::error_code());
   }
   SM.setMainFileID(SM.createFileID(*FE, SourceLocation(), SrcMgr::C_User));
   ParseAST(CI.getPreprocessor(), , CI.getASTContext());
@@ -360,7 +361,7 @@ int main(int argc, const char **argv) {
   for (auto I : Imports) {
 llvm::Expected ImportCI = Parse(I, {}, false, false);
 if (auto E = ImportCI.takeError()) {
-  llvm::errs() << llvm::toString(std::move(E));
+  llvm::errs() << "error: " << llvm::toString(std::move(E)) << "\n";
   exit(-1);
 }
 ImportCIs.push_back(std::move(*ImportCI));
@@ -379,7 +380,7 @@ int main(int argc, const char **argv) {
   Parse(Expression, (Direct && !UseOrigins) ? ImportCIs : IndirectCIs,
 DumpAST, DumpIR);
   if (auto E = ExpressionCI.takeError()) {
-llvm::errs() << llvm::toString(std::move(E));
+llvm::errs() << "error: " << llvm::toString(std::move(E)) << "\n";
 exit(-1);
   }
   Forget(*ExpressionCI, (Direct && !UseOrigins) ? ImportCIs : IndirectCIs);



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

[PATCH] D92968: Frontend: Migrate to FileEntryRef in TextDiagnosticTest, NFC

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa60043219907: Frontend: Migrate to FileEntryRef in 
TextDiagnosticTest, NFC (authored by dexonsmith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92968

Files:
  clang/unittests/Frontend/TextDiagnosticTest.cpp


Index: clang/unittests/Frontend/TextDiagnosticTest.cpp
===
--- clang/unittests/Frontend/TextDiagnosticTest.cpp
+++ clang/unittests/Frontend/TextDiagnosticTest.cpp
@@ -46,7 +46,7 @@
   // Create a dummy file with some contents to produce a test SourceLocation.
   const llvm::StringRef file_path = "main.cpp";
   const llvm::StringRef main_file_contents = "some\nsource\ncode\n";
-  const clang::FileEntry  = *FileMgr.getVirtualFile(
+  const clang::FileEntryRef fe = FileMgr.getVirtualFileRef(
   file_path,
   /*Size=*/static_cast(main_file_contents.size()),
   /*ModificationTime=*/0);
@@ -55,11 +55,11 @@
   buffer.append(main_file_contents.begin(), main_file_contents.end());
   auto file_contents = std::make_unique(
   std::move(buffer), file_path);
-  SrcMgr.overrideFileContents(, std::move(file_contents));
+  SrcMgr.overrideFileContents(fe, std::move(file_contents));
 
   // Create the actual file id and use it as the main file.
   clang::FileID fid =
-  SrcMgr.createFileID(, SourceLocation(), clang::SrcMgr::C_User);
+  SrcMgr.createFileID(fe, SourceLocation(), clang::SrcMgr::C_User);
   SrcMgr.setMainFileID(fid);
 
   // Create the source location for the test diagnostic.


Index: clang/unittests/Frontend/TextDiagnosticTest.cpp
===
--- clang/unittests/Frontend/TextDiagnosticTest.cpp
+++ clang/unittests/Frontend/TextDiagnosticTest.cpp
@@ -46,7 +46,7 @@
   // Create a dummy file with some contents to produce a test SourceLocation.
   const llvm::StringRef file_path = "main.cpp";
   const llvm::StringRef main_file_contents = "some\nsource\ncode\n";
-  const clang::FileEntry  = *FileMgr.getVirtualFile(
+  const clang::FileEntryRef fe = FileMgr.getVirtualFileRef(
   file_path,
   /*Size=*/static_cast(main_file_contents.size()),
   /*ModificationTime=*/0);
@@ -55,11 +55,11 @@
   buffer.append(main_file_contents.begin(), main_file_contents.end());
   auto file_contents = std::make_unique(
   std::move(buffer), file_path);
-  SrcMgr.overrideFileContents(, std::move(file_contents));
+  SrcMgr.overrideFileContents(fe, std::move(file_contents));
 
   // Create the actual file id and use it as the main file.
   clang::FileID fid =
-  SrcMgr.createFileID(, SourceLocation(), clang::SrcMgr::C_User);
+  SrcMgr.createFileID(fe, SourceLocation(), clang::SrcMgr::C_User);
   SrcMgr.setMainFileID(fid);
 
   // Create the source location for the test diagnostic.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a600432 - Frontend: Migrate to FileEntryRef in TextDiagnosticTest, NFC

[Duncan P. N. Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2020-12-11T17:06:28-08:00
New Revision: a60043219907b8f370263b7d3d4827b83388d8cf

URL: 
https://github.com/llvm/llvm-project/commit/a60043219907b8f370263b7d3d4827b83388d8cf
DIFF: 
https://github.com/llvm/llvm-project/commit/a60043219907b8f370263b7d3d4827b83388d8cf.diff

LOG: Frontend: Migrate to FileEntryRef in TextDiagnosticTest, NFC

Migrate over to the `FileEntryRef` overloads of
`SourceManager::createFileID` and `overrideFileContents` (using
`getVirtualFileRef`) in `TextDiagnostic`'s `ShowLine` test.

No functionality change.

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

Added: 


Modified: 
clang/unittests/Frontend/TextDiagnosticTest.cpp

Removed: 




diff  --git a/clang/unittests/Frontend/TextDiagnosticTest.cpp 
b/clang/unittests/Frontend/TextDiagnosticTest.cpp
index 1e05104d9388..220cff6643d5 100644
--- a/clang/unittests/Frontend/TextDiagnosticTest.cpp
+++ b/clang/unittests/Frontend/TextDiagnosticTest.cpp
@@ -46,7 +46,7 @@ TEST(TextDiagnostic, ShowLine) {
   // Create a dummy file with some contents to produce a test SourceLocation.
   const llvm::StringRef file_path = "main.cpp";
   const llvm::StringRef main_file_contents = "some\nsource\ncode\n";
-  const clang::FileEntry  = *FileMgr.getVirtualFile(
+  const clang::FileEntryRef fe = FileMgr.getVirtualFileRef(
   file_path,
   /*Size=*/static_cast(main_file_contents.size()),
   /*ModificationTime=*/0);
@@ -55,11 +55,11 @@ TEST(TextDiagnostic, ShowLine) {
   buffer.append(main_file_contents.begin(), main_file_contents.end());
   auto file_contents = std::make_unique(
   std::move(buffer), file_path);
-  SrcMgr.overrideFileContents(, std::move(file_contents));
+  SrcMgr.overrideFileContents(fe, std::move(file_contents));
 
   // Create the actual file id and use it as the main file.
   clang::FileID fid =
-  SrcMgr.createFileID(, SourceLocation(), clang::SrcMgr::C_User);
+  SrcMgr.createFileID(fe, SourceLocation(), clang::SrcMgr::C_User);
   SrcMgr.setMainFileID(fid);
 
   // Create the source location for the test diagnostic.



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

[PATCH] D93148: Basic: Add native support for stdin to SourceManager and FileManager

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.
dexonsmith added reviewers: arphaman, JDevlieghere, jansvoboda11.
Herald added a subscriber: ributzka.
dexonsmith requested review of this revision.
Herald added a project: clang.

Add support for stdin to SourceManager and FileManager. Adds
FileManager::getSTDIN, which adds a FileEntryRef for `` and reads
the MemoryBuffer, which is stored as `FileEntry::Content`.

Eventually the other buffers in `ContentCache` will sink to here as well

- we probably usually want to load/save a MemoryBuffer eagerly -- but

it's happening early for stdin to get rid of
CompilerInstance::InitializeSourceManager's final call to
`SourceManager::overrideFileContents`.

clang/test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.export/p1.cpp
relies on building a module from stdin; supporting that requires setting
ContentCache::BufferOverridden.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93148

Files:
  clang/include/clang/Basic/FileEntry.h
  clang/include/clang/Basic/FileManager.h
  clang/lib/Basic/FileEntry.cpp
  clang/lib/Basic/FileManager.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Frontend/CompilerInstance.cpp

Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -848,33 +848,22 @@
   StringRef InputFile = Input.getFile();
 
   // Figure out where to get and map in the main file.
-  if (InputFile != "-") {
-auto FileOrErr = FileMgr.getFileRef(InputFile, /*OpenFile=*/true);
-if (!FileOrErr) {
-  // FIXME: include the error in the diagnostic.
-  consumeError(FileOrErr.takeError());
+  auto FileOrErr = InputFile == "-"
+   ? FileMgr.getSTDIN()
+   : FileMgr.getFileRef(InputFile, /*OpenFile=*/true);
+  if (!FileOrErr) {
+// FIXME: include the error in the diagnostic even when it's not stdin.
+auto EC = llvm::errorToErrorCode(FileOrErr.takeError());
+if (InputFile != "-")
   Diags.Report(diag::err_fe_error_reading) << InputFile;
-  return false;
-}
-
-SourceMgr.setMainFileID(
-SourceMgr.createFileID(*FileOrErr, SourceLocation(), Kind));
-  } else {
-llvm::ErrorOr> SBOrErr =
-llvm::MemoryBuffer::getSTDIN();
-if (std::error_code EC = SBOrErr.getError()) {
+else
   Diags.Report(diag::err_fe_error_reading_stdin) << EC.message();
-  return false;
-}
-std::unique_ptr SB = std::move(SBOrErr.get());
-
-FileEntryRef File = FileMgr.getVirtualFileRef(SB->getBufferIdentifier(),
-  SB->getBufferSize(), 0);
-SourceMgr.setMainFileID(
-SourceMgr.createFileID(File, SourceLocation(), Kind));
-SourceMgr.overrideFileContents(File, std::move(SB));
+return false;
   }
 
+  SourceMgr.setMainFileID(
+  SourceMgr.createFileID(*FileOrErr, SourceLocation(), Kind));
+
   assert(SourceMgr.getMainFileID().isValid() &&
  "Couldn't establish MainFileID!");
   return true;
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -420,6 +420,7 @@
 
   Entry->IsFileVolatile = UserFilesAreVolatile && !isSystemFile;
   Entry->IsTransient = FilesAreTransient;
+  Entry->BufferOverridden |= FileEnt.isNamedPipe();
 
   return *Entry;
 }
Index: clang/lib/Basic/FileManager.cpp
===
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -338,6 +338,24 @@
   return ReturnedRef;
 }
 
+llvm::Expected FileManager::getSTDIN() {
+  // Only read stdin once.
+  if (STDIN)
+return *STDIN;
+
+  std::unique_ptr Content;
+  if (auto ContentOrError = llvm::MemoryBuffer::getSTDIN())
+Content = std::move(*ContentOrError);
+  else
+return llvm::errorCodeToError(ContentOrError.getError());
+
+  STDIN = getVirtualFileRef("", Content->getBufferSize(), 0);
+  FileEntry  = const_cast(STDIN->getFileEntry());
+  FE.Content = std::move(Content);
+  FE.IsNamedPipe = true;
+  return *STDIN;
+}
+
 const FileEntry *FileManager::getVirtualFile(StringRef Filename, off_t Size,
  time_t ModificationTime) {
   return (Filename, Size, ModificationTime).getFileEntry();
@@ -486,6 +504,10 @@
 llvm::ErrorOr>
 FileManager::getBufferForFile(const FileEntry *Entry, bool isVolatile,
   bool RequiresNullTerminator) {
+  // If the content is living on the file entry, return a reference to it.
+  if (Entry->Content)
+return llvm::MemoryBuffer::getMemBuffer(Entry->Content->getMemBufferRef());
+
   uint64_t FileSize = Entry->getSize();
   // If there's a high enough chance that the file have changed since we
   // got its size, force a stat before opening it.
Index: clang/lib/Basic/FileEntry.cpp

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Miya Natsuhara via Phabricator via cfe-commits]

mnatsuhara added a comment.

After following up with the team responsible for 
`__builtin_zero_non_value_bits` on MSVC, I have some more information to offer 
in conjunction with @BillyONeal's report:

- For unions, we always assume that it has unique object representations and 
thus does not have any padding bytes.  This allows 
`__builtin_zero_non_value_bits` to be used with types with union members, not 
always accurately but never destructively.  (If the union has padding bytes, we 
don't know which member is active so we don't know exactly where the padding 
bytes are.  To return padding bytes assuming any member risks changing the 
value.)
- Our implementation does appear to include tail padding and this would include 
any such padding that is due to alignment requirements, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[PATCH] D93104: [clang][cli] Revert accidental access-control flag rename

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

Thanks! Sorry for the breakage.

Actually, neither `-fno-access-control` nor `-faccess-control` has any test..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93104

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

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

This also crashes test-suite, so I've reverted the change for now. (Stack trace 
for tramp3d-v4 crash: 
https://llvm-compile-time-tracker.com/show_error.php?commit=7b3470baf8bab1919e3ad4c18e2b776c1f7be2d5)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

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

[clang] 8d4b139 - Revert "Consider reference, pointer, and pointer-to-member TemplateArguments to be different if they have different types."

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2020-12-12T00:04:10+01:00
New Revision: 8d4b139e9dceb43aa91d0451f5458fd05a9fba33

URL: 
https://github.com/llvm/llvm-project/commit/8d4b139e9dceb43aa91d0451f5458fd05a9fba33
DIFF: 
https://github.com/llvm/llvm-project/commit/8d4b139e9dceb43aa91d0451f5458fd05a9fba33.diff

LOG: Revert "Consider reference, pointer, and pointer-to-member 
TemplateArguments to be different if they have different types."

This reverts commit 7b3470baf8bab1919e3ad4c18e2b776c1f7be2d5.

Causes a crash while building tramp3d-v4 from test-suite.

Added: 


Modified: 
clang/include/clang/Basic/LangOptions.h
clang/lib/AST/ItaniumMangle.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/AST/TemplateBase.cpp
clang/test/CodeGenCXX/clang-abi-compat.cpp
clang/test/CodeGenCXX/mangle-class-nttp.cpp
clang/test/CodeGenCXX/mangle-template.cpp
clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index 251c9a9ecb5d..203c45fdd9a7 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -155,10 +155,8 @@ class LangOptions : public LangOptionsBase {
 
 /// Attempt to be ABI-compatible with code generated by Clang 11.0.x
 /// (git  2e10b7a39b93). This causes clang to pass unions with a 256-bit
-/// vector member on the stack instead of using registers, to not properly
-/// mangle substitutions for template names in some cases, and to mangle
-/// declaration template arguments without a cast to the parameter type
-/// even when that can lead to mangling collisions.
+/// vector member on the stack instead of using registers, and to not
+/// properly mangle substitutions for template names in some cases.
 Ver11,
 
 /// Conform to the underlying platform's C and C++ ABIs as closely

diff  --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index 77ecba664f5b..f5a4f6708c83 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -551,15 +551,13 @@ class CXXNameMangler {
   void mangleCXXCtorType(CXXCtorType T, const CXXRecordDecl *InheritedFrom);
   void mangleCXXDtorType(CXXDtorType T);
 
-  void mangleTemplateArgs(TemplateName TN,
-  const TemplateArgumentLoc *TemplateArgs,
+  void mangleTemplateArgs(const TemplateArgumentLoc *TemplateArgs,
   unsigned NumTemplateArgs);
-  void mangleTemplateArgs(TemplateName TN, const TemplateArgument 
*TemplateArgs,
+  void mangleTemplateArgs(const TemplateArgument *TemplateArgs,
   unsigned NumTemplateArgs);
-  void mangleTemplateArgs(TemplateName TN, const TemplateArgumentList );
-  void mangleTemplateArg(TemplateArgument A, bool NeedExactType);
-  void mangleValueInTemplateArg(QualType T, const APValue , bool TopLevel,
-bool NeedExactType = false);
+  void mangleTemplateArgs(const TemplateArgumentList );
+  void mangleTemplateArg(TemplateArgument A);
+  void mangleValueInTemplateArg(QualType T, const APValue );
 
   void mangleTemplateParameter(unsigned Depth, unsigned Index);
 
@@ -825,11 +823,6 @@ isTemplate(GlobalDecl GD, const TemplateArgumentList 
*) {
   return GlobalDecl();
 }
 
-static TemplateName asTemplateName(GlobalDecl GD) {
-  const TemplateDecl *TD = dyn_cast_or_null(GD.getDecl());
-  return TemplateName(const_cast(TD));
-}
-
 void CXXNameMangler::mangleName(GlobalDecl GD) {
   const NamedDecl *ND = cast(GD.getDecl());
   if (const VarDecl *VD = dyn_cast(ND)) {
@@ -906,7 +899,7 @@ void CXXNameMangler::mangleNameWithAbiTags(GlobalDecl GD,
 const TemplateArgumentList *TemplateArgs = nullptr;
 if (GlobalDecl TD = isTemplate(GD, TemplateArgs)) {
   mangleUnscopedTemplateName(TD, AdditionalAbiTags);
-  mangleTemplateArgs(asTemplateName(TD), *TemplateArgs);
+  mangleTemplateArgs(*TemplateArgs);
   return;
 }
 
@@ -959,7 +952,7 @@ void CXXNameMangler::mangleTemplateName(const TemplateDecl 
*TD,
 
   if (DC->isTranslationUnit() || isStdNamespace(DC)) {
 mangleUnscopedTemplateName(TD, nullptr);
-mangleTemplateArgs(asTemplateName(TD), TemplateArgs, NumTemplateArgs);
+mangleTemplateArgs(TemplateArgs, NumTemplateArgs);
   } else {
 mangleNestedName(TD, TemplateArgs, NumTemplateArgs);
   }
@@ -1109,8 +1102,7 @@ void CXXNameMangler::manglePrefix(QualType type) {
   // FIXME: GCC does not appear to mangle the template arguments when
   // the template in question is a dependent template name. Should we
   // emulate that badness?
-  mangleTemplateArgs(TST->getTemplateName(), TST->getArgs(),
- TST->getNumArgs());
+  mangleTemplateArgs(TST->getArgs(), TST->getNumArgs());
   addSubstitution(QualType(TST, 0));
 }
   } else if (const auto *DTST 

[PATCH] D92191: [clang-scan-deps] Add support for clang-cl

[Michael Spencer via Phabricator via cfe-commits]

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

lgtm. Thanks for the patch.


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

https://reviews.llvm.org/D92191

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

[PATCH] D93104: [clang][cli] Revert accidental access-control flag rename

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a reviewer: MaskRay.
dexonsmith added a comment.

In D93104#2449787 , @dexonsmith wrote:

> In D93104#2449783 , @dexonsmith 
> wrote:
>
>> LGTM. BTW, for partial reverts or potentially time-sensitive fixes like 
>> this, it's usually better to land first and ask questions later.

Ah, I see the original commit was a while ago, so maybe not so time-sensitive.

> (It'd be nice to have a test for this too if possible...)

(Or maybe OptOutFFLag could reject an option beginning with `"no-"`?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93104

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

[PATCH] D93104: [clang][cli] Revert accidental access-control flag rename

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D93104#2449783 , @dexonsmith wrote:

> LGTM. BTW, for partial reverts or potentially time-sensitive fixes like this, 
> it's usually better to land first and ask questions later.

(It'd be nice to have a test for this too if possible...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93104

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

[PATCH] D93104: [clang][cli] Revert accidental access-control flag rename

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM. BTW, for partial reverts or potentially time-sensitive fixes like this, 
it's usually better to land first and ask questions later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93104

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

[PATCH] D83979: [clang][cli] Port LangOpts option flags to new option parsing system

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Driver/Options.td:1292-1303
 def fdwarf_exceptions : Flag<["-"], "fdwarf-exceptions">, Group,
-  Flags<[CC1Option]>, HelpText<"Use DWARF style exceptions">;
+  Flags<[CC1Option]>, HelpText<"Use DWARF style exceptions">,
+  MarshallingInfoFlag<"LangOpts->DWARFExceptions">;
 def fsjlj_exceptions : Flag<["-"], "fsjlj-exceptions">, Group,
-  Flags<[CC1Option]>, HelpText<"Use SjLj style exceptions">;
+  Flags<[CC1Option]>, HelpText<"Use SjLj style exceptions">,
+  MarshallingInfoFlag<"LangOpts->SjLjExceptions">;
 def fseh_exceptions : Flag<["-"], "fseh-exceptions">, Group,

These options should be mutually exclusive -- as in, the last flag wins -- but 
I don't see how that's implemented now (the previous logic was via 
`getLastArg`). If that is working properly, can you explain how?

If it's not working right now, my suggestion would be to separate these options 
out to do in a separate patch series. I would suggest, rather than modelling 
the current behaviour, we leverage our flexibility to change `-cc1` options 
(e.g., could do three patches, where the first adds accessors to LangOpts and 
updates all users, the second changes the keypath to a single `ExceptionStyle` 
enum, and then the third patch changes the `-cc1` option to `-fexception-style` 
and starts using the marshalling infrastructure).



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:293
 static T mergeForwardValue(T KeyPath, U Value) {
-  return Value;
+  return static_cast(Value);
 }

These nits might be better to do in a follow-up, which also updated 
`extractForwardValue`, but since I'm seeing it now:
- Should this use `std::move`?
- Can we drop the `KeyPath` name?
```
template 
static T mergeForwardValue(T /*KeyPath*/, U Value) {
  return static_cast(std::move(Value));
}
```



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3655-3666
   llvm::Triple T(Res.getTargetOpts().Triple);
   if (DashX.getFormat() == InputKind::Precompiled ||
   DashX.getLanguage() == Language::LLVM_IR) {
-// ObjCAAutoRefCount and Sanitize LangOpts are used to setup the
-// PassManager in BackendUtil.cpp. They need to be initializd no matter
-// what the input type is.
-if (Args.hasArg(OPT_fobjc_arc))

Previously, these arguments were only claimed depending on `-x`; are we 
changing to claim these all the time? If so, that should be considered 
separately; I think in general we may want the ability to do claim some options 
only conditionally; @Bigcheese , any thoughts here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83979

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

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

Clang is crashing after this change when bootstrapping and building 
lib/Support/CMakeFiles/LLVMSupport.dir/Hashing.cpp.o
(our log if it helps: 
https://buildkite.com/mlir/mlir-core/builds/10003#80a5b6a9-d8d9-4420-ab2a-c24f398bc5d4
 )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

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

[PATCH] D92774: [clang][cli] CompilerInvocationTest: add tests for boolean options

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92774

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

[PATCH] D92857: [clang][cli] Don't always emit -f[no-]legacy-pass-manager

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.

LGTM too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92857

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

[PATCH] D92775: [clang][cli] Add flexible TableGen multiclass for boolean options

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.

(LGTM, in case my conditional approval earlier wasn't clear...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92775

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

[PATCH] D93094: [clang][cli] Prevent double denormalization

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.



Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:267
 
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fdebug-pass-manager")));
+  ASSERT_EQ(count(GeneratedArgs, "-fdebug-pass-manager"), 1);
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fno-debug-pass-manager";

jansvoboda11 wrote:
> Is it wise to rely on pointer comparison here? The call to `count` returns 2 
> before changing the denormalizer and 1 after, but I'm not sure if it will 
> work on all platforms.
Does this compile / avoid the pointer comparison?
```
ASSERT_EQ(count(GeneratedArgs, StringRef("-fdebug-pass-manager")), 1);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93094

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

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b3470baf8ba: Consider reference, pointer, and 
pointer-to-member TemplateArguments to be… (authored by rsmith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/test/CodeGenCXX/clang-abi-compat.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/mangle-template.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -187,6 +187,11 @@
 int  = f(B<>());
 float  = f(B<>());
 
+void type_affects_identity(B<>) {}
+void type_affects_identity(B<(const int*)>) {}
+void type_affects_identity(B<(void*)>) {}
+void type_affects_identity(B<(const void*)>) {}
+
 // pointers to members
 template struct B {};
 template struct B {};
@@ -198,6 +203,12 @@
 char  = f(B<::p>());
 short  = f(B<::pp>());
 
+struct Y : X {};
+void type_affects_identity(B<::n>) {}
+void type_affects_identity(B<(int Y::*)::n>) {} // FIXME: expected-error {{sorry}}
+void type_affects_identity(B<(const int X::*)::n>) {}
+void type_affects_identity(B<(const int Y::*)::n>) {} // FIXME: expected-error {{sorry}}
+
 // A case where we need to do auto-deduction, and check whether the
 // resulting dependent types match during partial ordering. These
 // templates are not ordered due to the mismatching function parameter.
Index: clang/test/CodeGenCXX/mangle-template.cpp
===
--- clang/test/CodeGenCXX/mangle-template.cpp
+++ clang/test/CodeGenCXX/mangle-template.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++11 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+// RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++20 -emit-llvm -triple x86_64-linux-gnu -o - %s | FileCheck %s --check-prefixes=CHECK,CXX20
 // expected-no-diagnostics
 
 namespace test1 {
@@ -221,3 +222,78 @@
   void g() { f(1, 2); }
 }
 
+#if __cplusplus >= 202002L
+namespace cxx20 {
+  template struct A {};
+  template struct B {};
+
+  int x;
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1xE(
+  void f(A<>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKiadL_ZNS_1xE(
+  void f(A<(const int*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPvadL_ZNS_1xE(
+  void f(A<(void*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPvXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKvadL_ZNS_1xE(
+  void f(A<(const void*)>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKvXadL_ZNS_1xE(
+  void f(B) {}
+
+  struct Q { int x; };
+
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1Q1xE(
+  void f(A<::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEiXadL_ZNS1_1xE
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvMNS_1QEKiadL_ZNS1_1xE(
+  void f(A<(const int Q::*)::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEKiXadL_ZNS1_1xE(
+  void f(B) {}
+}
+#endif
+
+namespace test17 {
+  // Ensure we mangle the types for non-type template arguments if we've lost
+  // track of argument / parameter correspondence.
+  template struct X {};
+
+  // CHECK: define {{.*}} @_ZN6test171fILi1EJLi2ELi3ELi4vNS_1XIXT_EJLi5EXspT0_ELi6
+  template void f(X) {}
+  void g() { f<1, 2, 3, 4>({}); }
+
+  // Note: there is no J...E here, because we can't form a pack argument, and
+  // the 5u and 6u are mangled with the original type 'j' (unsigned int) not
+  // with the resolved type 'i' (signed int).
+  // CHECK: define {{.*}} @_ZN6test171hILi4EJLi1ELi2ELi3vNS_1XIXspT0_EXLj5EEXT_EXLj6
+  template void h(X) {}
+  void i() { h<4, 1, 2, 3>({}); }
+
+#if __cplusplus >= 201402L
+  template struct Y {};
+  int n;
+  // Case 1:  is a resolved template argument, with a known parameter:
+  // mangled with no conversion.
+  // CXX20: define {{.*}} @_ZN6test172j1ILi1EEEvNS_1YIXT_EXadL_ZNS_1nE
+  template void j1(Y) {}
+  // Case 2:  is an unresolved template argument, with an unknown
+  // corresopnding parameter: mangled as the source expression.
+  // CXX20: define {{.*}} @_ZN6test172j2IJLi1vNS_1YIXspT_EXcvPKiadL_ZNS_1nE
+  template void j2(Y) {}
+  // Case 3:  is a resolved 

[clang] 7b3470b - Consider reference, pointer, and pointer-to-member TemplateArguments to be different if they have different types.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-12-11T13:26:33-08:00
New Revision: 7b3470baf8bab1919e3ad4c18e2b776c1f7be2d5

URL: 
https://github.com/llvm/llvm-project/commit/7b3470baf8bab1919e3ad4c18e2b776c1f7be2d5
DIFF: 
https://github.com/llvm/llvm-project/commit/7b3470baf8bab1919e3ad4c18e2b776c1f7be2d5.diff

LOG: Consider reference, pointer, and pointer-to-member TemplateArguments to be 
different if they have different types.

For the Itanium ABI, this implements the mangling rule suggested in
https://github.com/itanium-cxx-abi/cxx-abi/issues/47, namely mangling
such template arguments as being cast to the parameter type in the case
where the template name is overloadable. This can cause a mangling
change for rare cases, where

 * the template argument declaration is converted from its declared type
   to the type of the template parameter, and
 * the template parameter either has a deduced type or is a parameter of
   a function template.

However, such changes are necessary to avoid mangling collisions. The
ABI changes can be reversed with -fclang-abi-compat=11 or earlier.

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

Added: 


Modified: 
clang/include/clang/Basic/LangOptions.h
clang/lib/AST/ItaniumMangle.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/AST/TemplateBase.cpp
clang/test/CodeGenCXX/clang-abi-compat.cpp
clang/test/CodeGenCXX/mangle-class-nttp.cpp
clang/test/CodeGenCXX/mangle-template.cpp
clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index 203c45fdd9a7..251c9a9ecb5d 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -155,8 +155,10 @@ class LangOptions : public LangOptionsBase {
 
 /// Attempt to be ABI-compatible with code generated by Clang 11.0.x
 /// (git  2e10b7a39b93). This causes clang to pass unions with a 256-bit
-/// vector member on the stack instead of using registers, and to not
-/// properly mangle substitutions for template names in some cases.
+/// vector member on the stack instead of using registers, to not properly
+/// mangle substitutions for template names in some cases, and to mangle
+/// declaration template arguments without a cast to the parameter type
+/// even when that can lead to mangling collisions.
 Ver11,
 
 /// Conform to the underlying platform's C and C++ ABIs as closely

diff  --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index f5a4f6708c83..77ecba664f5b 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -551,13 +551,15 @@ class CXXNameMangler {
   void mangleCXXCtorType(CXXCtorType T, const CXXRecordDecl *InheritedFrom);
   void mangleCXXDtorType(CXXDtorType T);
 
-  void mangleTemplateArgs(const TemplateArgumentLoc *TemplateArgs,
+  void mangleTemplateArgs(TemplateName TN,
+  const TemplateArgumentLoc *TemplateArgs,
   unsigned NumTemplateArgs);
-  void mangleTemplateArgs(const TemplateArgument *TemplateArgs,
+  void mangleTemplateArgs(TemplateName TN, const TemplateArgument 
*TemplateArgs,
   unsigned NumTemplateArgs);
-  void mangleTemplateArgs(const TemplateArgumentList );
-  void mangleTemplateArg(TemplateArgument A);
-  void mangleValueInTemplateArg(QualType T, const APValue );
+  void mangleTemplateArgs(TemplateName TN, const TemplateArgumentList );
+  void mangleTemplateArg(TemplateArgument A, bool NeedExactType);
+  void mangleValueInTemplateArg(QualType T, const APValue , bool TopLevel,
+bool NeedExactType = false);
 
   void mangleTemplateParameter(unsigned Depth, unsigned Index);
 
@@ -823,6 +825,11 @@ isTemplate(GlobalDecl GD, const TemplateArgumentList 
*) {
   return GlobalDecl();
 }
 
+static TemplateName asTemplateName(GlobalDecl GD) {
+  const TemplateDecl *TD = dyn_cast_or_null(GD.getDecl());
+  return TemplateName(const_cast(TD));
+}
+
 void CXXNameMangler::mangleName(GlobalDecl GD) {
   const NamedDecl *ND = cast(GD.getDecl());
   if (const VarDecl *VD = dyn_cast(ND)) {
@@ -899,7 +906,7 @@ void CXXNameMangler::mangleNameWithAbiTags(GlobalDecl GD,
 const TemplateArgumentList *TemplateArgs = nullptr;
 if (GlobalDecl TD = isTemplate(GD, TemplateArgs)) {
   mangleUnscopedTemplateName(TD, AdditionalAbiTags);
-  mangleTemplateArgs(*TemplateArgs);
+  mangleTemplateArgs(asTemplateName(TD), *TemplateArgs);
   return;
 }
 
@@ -952,7 +959,7 @@ void CXXNameMangler::mangleTemplateName(const TemplateDecl 
*TD,
 
   if (DC->isTranslationUnit() || isStdNamespace(DC)) {
 mangleUnscopedTemplateName(TD, nullptr);
-mangleTemplateArgs(TemplateArgs, NumTemplateArgs);
+mangleTemplateArgs(asTemplateName(TD), TemplateArgs, 

[PATCH] D92427: [OPENMP51] Add present modifier in defaultmap clause

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added inline comments.



Comment at: clang/test/OpenMP/target_defaultmap_codegen_01.cpp:1523
 
-// CK26-DAG: [[SIZES:@.+]] = {{.+}}constant [3 x i64] [i64 4096, i64 4, i64 
{{.+}}]
+// CK26-DAG: [[SIZES:@.+]] = {{.+}}constant [3 x i64] [i64 4, i64 4096, i64 
{{.+}}]
 // Map types: OMP_MAP_TO | MAP_ALWAYS | OMP_MAP_IMPLICIT = 533

ABataev wrote:
> Why the order has changed?
This is because, in SemaOpenMP line 5176, I'm invoking `ActOnOpenMPMapClause` 
in a fixed order. The outer dimension of the 2-dim for loop is the variable 
category of defaultmap (scalar -> aggregate -> pointer), therefore, 4096 
(aggregate, Vector) is not happening after 4 (scalar, a).



Comment at: clang/test/OpenMP/target_defaultmap_codegen_01.cpp:1524
+// CK26-DAG: [[SIZES:@.+]] = {{.+}}constant [3 x i64] [i64 4, i64 4096, i64 
{{.+}}]
 // Map types: OMP_MAP_TO | MAP_ALWAYS | OMP_MAP_IMPLICIT = 533
 // CK26-DAG: [[TYPES:@.+]] = {{.+}}constant [3 x i64] [i64 531, i64 531, i64 
531]

ABataev wrote:
> Looks like this comment is outdated and must be fixed.
Going to fix this, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92427

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

[PATCH] D91261: [OPENMP]Do not use OMP_MAP_TARGET_PARAM for data movement directives.

[Chi Chun Chen via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91261

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

[PATCH] D92160: [clang] Fix wrong FDs are used for files with same name in Tooling

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D92160#2447861 , @OikawaKirie wrote:

> Replies from the original author Hao Zhang 
>
>   llvm::ErrorOr
>   FileManager::getFile(StringRef Filename, bool openFile, bool CacheFailure) {
> // Froce using the absolute path to query the cache
> llvm::SmallString<128> AbsPath(Filename);
> makeAbsolutePath(AbsPath);
> auto Result = getFileRef(AbsPath.str(), openFile, CacheFailure);
> if (Result)
>   return >getFileEntry();
> return llvm::errorToErrorCode(Result.takeError());
>   }

(A quibble here is that `getFile` is just a wrapper around `getFileRef`, and 
might eventually be removed; certainly `FileEntry::getName` is on its way out; 
you'd want this logic at the top of `getFileRef`,  `getVirtualFileRef`, etc.)

> It is more likely a defensive workaround to this problem. It might not be a 
> good solution to the real problem (the assumption that the CWD won't change). 
> The above code resulted in failures of some test cases, I haven't looked 
> closely into why they failed. In my point of view, `Filename` should only be 
> used as the key to query the cache. Logically, if I change it into the 
> absolute path, those test cases should pass as usual.

I think it would be good to understand why the tests failed.

Are some clients relying on the name matching the query? If so, why?

My guess, but I'm really not sure and it would be good to confirm, is that we 
need this to support reproducible builds, where you don't want the CWD to leak 
into your output. In this case, you'd run all build commands from the same 
directory, but invoke clang using all relative paths, perhaps something like 
this for your example:

  % clang -g -o a/a.o -Isrc/a src/a/a.c
  % clang -g -o b/b.o -Isrc/b src/b/b.c

Perhaps (I'm not sure) there are places in clang that rely on `FileManager` 
returning relative paths to support this kind of thing. If anything relied on 
it, my bet would be modules-related code.

But it's possible we don't need this. If it's safe for us to update the tests 
and make `FileManager::getFileRef` always canonicalize to an absolute path, 
that would definitely be cleaner. `FileManager::makeAbsolute` can use whatever 
the FS's CWD is at the time of query... nice and clean.

> Anyway, your approach (remove `FileSystemOptions`, one cache for absolute 
> paths, multiple caches for relative paths) looks good to me.

(But as you mentioned, not as clean as canonicalizing to absolute paths, if we 
can do it.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92160

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

[PATCH] D92432: [analyzer] Add a thin abstraction layer between libCrossTU and libAnalysis.

[Balázs Benics via Phabricator via cfe-commits]

steakhal added inline comments.



Comment at: clang/lib/Analysis/PlistPathDiagnosticConsumer.cpp:993
 MacroLoc = LocAndUnit->first;
 PPToUse = >second->getPreprocessor();
   }

NoQ wrote:
> We were reverted because there's still a dependency on `ASTUnit` which lives 
> in `libFrontend`. I guess i'll simplify the virtual method to return 
> `(MacroLoc, PPToUse)` directly (?) (the old method will still remain on 
> `CrossTranslationUnitContext`).
I don't see much uses of this function in the codebase. Why do you think it 
worth keeping it?

As a side-note, I'm planning to upstream some serious changes in macro 
expansion handling. Which will change the highlighted parts, probably resolving 
this circular dependency too.
Let me polish my work, and expect the patch on monday or tuesday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92432

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

[PATCH] D72241: [clang-tidy] new altera single work item barrier check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies marked 4 inline comments as done.
ffrankies added a comment.

@aaron.ballman Thank you! If there are no further comments, could you please 
commit this on my behalf? My GitHub username is ffrankies 
.




Comment at: 
clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.cpp:49
+  bool IsNDRange = false;
+  for (const Attr *Attribute : MatchedDecl->getAttrs()) {
+if (Attribute->getKind() == attr::Kind::ReqdWorkGroupSize) {

aaron.ballman wrote:
> Rather than getting all attributes, you can use `specific_attrs<>` to loop 
> over just the specific attributes you care about on the declaration. That 
> also fixes the non-idiomatic way to convert from an attribute to a specific 
> derived attribute (which should use `dyn_cast<>` or friends).
I went with `hasAttr<>` and then `getAttr<>` to avoid a loop over all 
attributes - there should only be one `ReqdWorkGroupSizeAttr`



Comment at: 
clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.cpp:71
+diag(MatchedDecl->getLocation(),
+ "Kernel function %0 does not call get_global_id or get_local_id may "
+ "be a viable single work-item kernel, but barrier call at %1 will "

aaron.ballman wrote:
> Same here as above.
> 
> Will users know what `NDRange execution` means? (I'm can't really understand 
> what the diagnostic is telling me, but this is outside of my typical domain.)
I discussed this with other students in my lab and the consensus there is this 
should be clear for OpenCL/OpenCL-for-FPGA users. If really needed, we can try 
to re-phrase it, but then the diagnostics will most likely become more wordy.


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

https://reviews.llvm.org/D72241

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

[PATCH] D93134: [FPEnv] Teach the IRBuilder about invoke's correct use of the strictfp attribute.

[Kevin P. Neal via Phabricator via cfe-commits]

kpn created this revision.
kpn added reviewers: spatel, rjmccall, andrew.w.kaylor, mibintc, sepavloff.
kpn requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Similar to D69312 , and documented in D69839 
, the IRBuilder needs to add the strictfp 
attribute to invoke instructions when constrained floating point is enabled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93134

Files:
  clang/test/CodeGen/exceptions-strictfp.c
  llvm/include/llvm/IR/IRBuilder.h


Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -301,7 +301,7 @@
 }
   }
 
-  void setConstrainedFPCallAttr(CallInst *I) {
+  void setConstrainedFPCallAttr(CallBase *I) {
 I->addAttribute(AttributeList::FunctionIndex, Attribute::StrictFP);
   }
 
@@ -1019,16 +1019,21 @@
ArrayRef Args,
ArrayRef OpBundles,
const Twine  = "") {
-return Insert(
-InvokeInst::Create(Ty, Callee, NormalDest, UnwindDest, Args, 
OpBundles),
-Name);
+InvokeInst *II =
+InvokeInst::Create(Ty, Callee, NormalDest, UnwindDest, Args, 
OpBundles);
+if (IsFPConstrained)
+  setConstrainedFPCallAttr(II);
+return Insert(II, Name);
   }
   InvokeInst *CreateInvoke(FunctionType *Ty, Value *Callee,
BasicBlock *NormalDest, BasicBlock *UnwindDest,
ArrayRef Args = None,
const Twine  = "") {
-return Insert(InvokeInst::Create(Ty, Callee, NormalDest, UnwindDest, Args),
-  Name);
+InvokeInst *II =
+InvokeInst::Create(Ty, Callee, NormalDest, UnwindDest, Args);
+if (IsFPConstrained)
+  setConstrainedFPCallAttr(II);
+return Insert(II, Name);
   }
 
   InvokeInst *CreateInvoke(FunctionCallee Callee, BasicBlock *NormalDest,
Index: clang/test/CodeGen/exceptions-strictfp.c
===
--- /dev/null
+++ clang/test/CodeGen/exceptions-strictfp.c
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -triple armv7-apple-unknown -ffp-exception-behavior=strict 
-fexperimental-strict-floating-point -emit-llvm -o - %s -fexceptions 
-fsjlj-exceptions -fblocks | FileCheck %s
+
+// Verify strictfp attributes on invoke calls (and therefore also on
+// function definitions).
+
+// rdar://problem/8621849
+void test1() {
+  extern void test1_helper(void (^)(int));
+
+  // CHECK: define arm_aapcscc void @test1() [[STRICTFP0:#[0-9]+]] personality 
i8* bitcast (i32 (...)* @__gcc_personality_sj0 to i8*)
+
+  __block int x = 10;
+
+  // CHECK: invoke arm_aapcscc void @test1_helper({{.*}}) [[STRICTFP1:#[0-9]+]]
+  test1_helper(^(int v) { x = v; });
+
+  // CHECK:  landingpad { i8*, i32 }
+  // CHECK-NEXT:   cleanup
+}
+
+void test2_helper();
+void test2() {
+  // CHECK: define arm_aapcscc void @test2() [[STRICTFP0]] personality i8* 
bitcast (i32 (...)* @__gcc_personality_sj0 to i8*) {
+  __block int x = 10;
+  ^{ (void)x; };
+
+  // CHECK: invoke arm_aapcscc void @test2_helper({{.*}}) [[STRICTFP1:#[0-9]+]]
+  test2_helper(5, 6, 7);
+
+  // CHECK:  landingpad { i8*, i32 }
+  // CHECK-NEXT:   cleanup
+}
+void test2_helper(int x, int y) {
+}
+
+// CHECK: attributes [[STRICTFP0]] = { {{.*}}strictfp{{.*}} }
+// CHECK: attributes [[STRICTFP1]] = { strictfp }


Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -301,7 +301,7 @@
 }
   }
 
-  void setConstrainedFPCallAttr(CallInst *I) {
+  void setConstrainedFPCallAttr(CallBase *I) {
 I->addAttribute(AttributeList::FunctionIndex, Attribute::StrictFP);
   }
 
@@ -1019,16 +1019,21 @@
ArrayRef Args,
ArrayRef OpBundles,
const Twine  = "") {
-return Insert(
-InvokeInst::Create(Ty, Callee, NormalDest, UnwindDest, Args, OpBundles),
-Name);
+InvokeInst *II =
+InvokeInst::Create(Ty, Callee, NormalDest, UnwindDest, Args, OpBundles);
+if (IsFPConstrained)
+  setConstrainedFPCallAttr(II);
+return Insert(II, Name);
   }
   InvokeInst *CreateInvoke(FunctionType *Ty, Value *Callee,
BasicBlock *NormalDest, BasicBlock *UnwindDest,
ArrayRef Args = None,
const Twine  = "") {
-return Insert(InvokeInst::Create(Ty, Callee, NormalDest, UnwindDest, Args),
-  Name);
+InvokeInst *II =
+InvokeInst::Create(Ty, Callee, NormalDest, UnwindDest, Args);
+if (IsFPConstrained)
+  setConstrainedFPCallAttr(II);
+return Insert(II, 

[PATCH] D92432: [analyzer] Add a thin abstraction layer between libCrossTU and libAnalysis.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/Analysis/PlistPathDiagnosticConsumer.cpp:993
 MacroLoc = LocAndUnit->first;
 PPToUse = >second->getPreprocessor();
   }

We were reverted because there's still a dependency on `ASTUnit` which lives in 
`libFrontend`. I guess i'll simplify the virtual method to return `(MacroLoc, 
PPToUse)` directly (?) (the old method will still remain on 
`CrossTranslationUnitContext`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92432

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

[PATCH] D84018: [clang][cli] Port Preprocessor and PreprocessorOutput option flags to new option parsing system

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84018

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

[PATCH] D84188: Port FileSystem options to new option parsing system

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84188

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

Re: [clang] ea66410 - Revert "Revert "Revert "Revert "[analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.""""

[Artem Dergachev via cfe-commits]

Hmm, indeed, thanks for reverting, i'll get this fixed.

Weird that i'm not getting any compile errors locally or from buildbots. 
Shared libs buildbots are usually very loud about this stuff.


On 12/11/20 12:51 AM, Haojian Wu wrote:

Hi Artem,

Looks like this patch has some layer violations, I tried to fix that 
but there is a main issue not easy to fix -- the issue is that there 
is a cycle dependency:


1. PlistPathDiagnosticConsumer.cpp (now being moved to clangAnalysis 
target), and this file calls a method from clang::ASTUnit, so 
*clangAnalysis* should depend on *clangFrontend* (this is not listed 
in the CMake file).

2. *clangFrontend* depends on *clangSema*
3. *clangSema* depends on *clangAnalysis*
*
*
I'm going to revert this patch (to unblock our integration, sorry), 
feel free to land it again if you fix the issue. Thanks!





On Thu, Dec 10, 2020 at 8:03 PM Artem Dergachev via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:



Author: Artem Dergachev
Date: 2020-12-10T11:02:54-08:00
New Revision: ea6641085d025ca0a5cef940465ef14d0ccace02

URL:

https://github.com/llvm/llvm-project/commit/ea6641085d025ca0a5cef940465ef14d0ccace02


DIFF:

https://github.com/llvm/llvm-project/commit/ea6641085d025ca0a5cef940465ef14d0ccace02.diff



LOG: Revert "Revert "Revert "Revert "[analyzer] NFC: Move path
diagnostic consumer implementations to libAnalysis.

This reverts commit 6a89cb8136f3435bd977b419b683dc0acc98e61e.

Added:
    clang/include/clang/Analysis/PathDiagnosticConsumers.def
    clang/include/clang/Analysis/PathDiagnosticConsumers.h
    clang/lib/Analysis/HTMLPathDiagnosticConsumer.cpp
    clang/lib/Analysis/PlistHTMLPathDiagnosticConsumer.cpp
    clang/lib/Analysis/PlistPathDiagnosticConsumer.cpp
    clang/lib/Analysis/SarifPathDiagnosticConsumer.cpp
    clang/lib/Analysis/TextPathDiagnosticConsumer.cpp

Modified:
    clang/include/clang/StaticAnalyzer/Core/Analyses.def
    clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
    clang/include/clang/module.modulemap
    clang/lib/Analysis/CMakeLists.txt
    clang/lib/Frontend/CompilerInvocation.cpp
    clang/lib/StaticAnalyzer/Core/CMakeLists.txt
    clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Removed:
clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h
    clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
    clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
    clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
    clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp




diff  --git
a/clang/include/clang/Analysis/PathDiagnosticConsumers.def
b/clang/include/clang/Analysis/PathDiagnosticConsumers.def
new file mode 100644
index ..33d2072fcf31
--- /dev/null
+++ b/clang/include/clang/Analysis/PathDiagnosticConsumers.def
@@ -0,0 +1,50 @@
+//===-- PathDiagnosticConsumers.def - Visualizing warnings
--*- 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
+//

+//===--===//
+//
+// This file defines the set of path diagnostic consumers -
objects that
+// implement
diff erent representations of static analysis results.
+//

+//===--===//
+
+#ifndef ANALYSIS_DIAGNOSTICS
+#define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)
+#endif
+
+ANALYSIS_DIAGNOSTICS(HTML, "html", "Output analysis results using
HTML",
+                     createHTMLDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(
+    HTML_SINGLE_FILE, "html-single-file",
+    "Output analysis results using HTML (not allowing for
multi-file bugs)",
+    createHTMLSingleFileDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(PLIST, "plist", "Output analysis results
using Plists",
+                     createPlistDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(
+    PLIST_MULTI_FILE, "plist-multi-file",
+    "Output analysis results using Plists (allowing for
multi-file bugs)",
+    createPlistMultiFileDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(PLIST_HTML, "plist-html",
+                     "Output analysis results using HTML wrapped
with Plists",
  

[PATCH] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

In D85788#2444136 , @guiand wrote:

> IMO it's better to just one-and-done programatically add `-Xclang 
> -disable-noundef-analysis` to all the tests' RUN lines. That way there are no 
> hidden flags.

If a script for this can be written and people who are working for the 
downstream project are happy with the script, this is the best approach IMO. It 
is clear and explicit.

In D85788#2441457 , @eugenis wrote:

>> This new substitution is going to be used for the noundef checks in D81678 
>>  only anyway.
>
> I'm not sure what you mean by this. The substitution is used ~380 times in 
> this patch alone.
> In the future, any attempt to use %clang -### or -%clang -cc1as will produce 
> the same unused argument warning, and it is very unclear that is can be fixed 
> with %clang_bin. Note that code search for "-disable-noundef-analysis" does 
> NOT lead you to the definition of %clang_bin!

I see, I was naively thinking the `%clang` -> `%clang_bin` changes were here to 
simply show that they are equivalent if function signatures are not checked. I 
thought `%clang` could be still used for those tests.
But they were here because the upcoming patch will otherwise make them raise 
unused argument warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85788

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

[PATCH] D92080: [Clang] Mutate long-double math builtins into f128 under IEEE-quad

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

In D92080#2448094 , @qiucf wrote:

> In D92080#244 , @craig.topper 
> wrote:
>
>> I don't think I understand the whole picture here. Why would only builtins 
>> get mutated? Does a call to "modf" still call "modf"? But __builtin_modf 
>> will call modf128? Is there a corresponding patch to the runtime libcalls 
>> table in the backend? With -ffast-math __builtin_sinl becomes llvm.sin.f128 
>> and the backend will emit a call to sinl if ISD::SIN isn't legal.
>
> We're going to make IEEE `f128` available in Clang on ppc64le. Newest glibc 
> already supports 'redirection' from `fmodl` to `fmodf128` (or, 
> `__fmodieee128`) by checking some macros determined by current long-double 
> semantics. For these LLVM intrinsics, Steven has a patch to fix the correct 
> signature of libcall (D91675 ). Builtin in 
> Clang will generate intrinsics under `ffast-math` but function call without 
> the option. That may lead to an awkward situation: program is correct under 
> fast-math, but wrong under no optimization. So such mutation is needed.
>
> This patch looks reasonable to me for targets with more than one long-double 
> format. Or if that's not a right way for the mutation, are there other places 
> to implement that?

D91675  has PowerPC only changes to make the 
f128 calls get emitted. If you change the frontend in a target independent way 
as proposed here, won't that make the frontend and backend not match for 
targets like X86?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92080

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

[PATCH] D91495: [clang-tidy] false-positive for bugprone-redundant-branch-condition in case of passed-by-ref params

[Zinovy Nis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG709112bce442: [clang-tidy] false-positive for 
bugprone-redundant-branch-condition in case of… (authored by zinovy.nis).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91495

Files:
  clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
@@ -6,7 +6,8 @@
 bool isBurning();
 bool isReallyBurning();
 bool isCollapsing();
-void tryToExtinguish(bool&);
+bool tryToExtinguish(bool&);
+bool tryToExtinguishByVal(bool);
 void tryPutFireOut();
 bool callTheFD();
 void scream();
@@ -948,6 +949,30 @@
   }
 }
 
+void negative_by_ref(bool onFire) {
+  if (tryToExtinguish(onFire) && onFire) {
+if (tryToExtinguish(onFire) && onFire) {
+  // NO-MESSAGE: fire may have been extinguished
+  scream();
+}
+  }
+}
+
+void negative_by_val(bool onFire) {
+  if (tryToExtinguishByVal(onFire) && onFire) {
+if (tryToExtinguish(onFire) && onFire) {
+  // NO-MESSAGE: fire may have been extinguished
+  scream();
+}
+  }
+  if (tryToExtinguish(onFire) && onFire) {
+if (tryToExtinguishByVal(onFire) && onFire) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'onFire' [bugprone-redundant-branch-condition]
+  scream();
+}
+  }
+}
+
 void negative_reassigned() {
   bool onFire = isBurning();
   if (onFire) {
@@ -1077,9 +1102,9 @@
 int positive_expr_with_cleanups() {
   class RetT {
   public:
-RetT(const int _code) : code_(_code) {}
-bool Ok() const { return code_ == 0; }
-static RetT Test(bool &_isSet) { return 0; }
+RetT(const int code);
+bool Ok() const;
+static RetT Test(bool isSet);
 
   private:
 int code_;
Index: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
@@ -23,15 +23,21 @@
 static const char CondVarStr[] = "cond_var";
 static const char OuterIfStr[] = "outer_if";
 static const char InnerIfStr[] = "inner_if";
+static const char OuterIfVar1Str[] = "outer_if_var1";
+static const char OuterIfVar2Str[] = "outer_if_var2";
+static const char InnerIfVar1Str[] = "inner_if_var1";
+static const char InnerIfVar2Str[] = "inner_if_var2";
 static const char FuncStr[] = "func";
 
-/// Returns whether `Var` is changed in `S` before `NextS`.
-static bool isChangedBefore(const Stmt *S, const Stmt *NextS,
+/// Returns whether `Var` is changed in range (`PrevS`..`NextS`).
+static bool isChangedBefore(const Stmt *S, const Stmt *NextS, const Stmt *PrevS,
 const VarDecl *Var, ASTContext *Context) {
   ExprMutationAnalyzer MutAn(*S, *Context);
   const auto  = Context->getSourceManager();
   const Stmt *MutS = MutAn.findMutation(Var);
   return MutS &&
+ SM.isBeforeInTranslationUnit(PrevS->getEndLoc(),
+  MutS->getBeginLoc()) &&
  SM.isBeforeInTranslationUnit(MutS->getEndLoc(), NextS->getBeginLoc());
 }
 
@@ -43,19 +49,22 @@
   Finder->addMatcher(
   ifStmt(
   hasCondition(ignoringParenImpCasts(anyOf(
-  declRefExpr(hasDeclaration(ImmutableVar)),
+  declRefExpr(hasDeclaration(ImmutableVar)).bind(OuterIfVar1Str),
   binaryOperator(hasOperatorName("&&"),
- hasEitherOperand(ignoringParenImpCasts(declRefExpr(
- hasDeclaration(ImmutableVar,
+ hasEitherOperand(ignoringParenImpCasts(
+ declRefExpr(hasDeclaration(ImmutableVar))
+ .bind(OuterIfVar2Str))),
   hasThen(hasDescendant(
   ifStmt(hasCondition(ignoringParenImpCasts(
- anyOf(declRefExpr(hasDeclaration(
-   varDecl(equalsBoundNode(CondVarStr,
+ anyOf(declRefExpr(hasDeclaration(varDecl(
+equalsBoundNode(CondVarStr
+.bind(InnerIfVar1Str),
binaryOperator(
hasAnyOperatorName("&&", "||"),
hasEitherOperand(ignoringParenImpCasts(

[clang-tools-extra] 709112b - [clang-tidy] false-positive for bugprone-redundant-branch-condition in case of passed-by-ref params

[Zinovy Nis via cfe-commits]

Author: Zinovy Nis
Date: 2020-12-11T21:09:51+03:00
New Revision: 709112bce4424a5436f0bb699c62b3fbc837fbb6

URL: 
https://github.com/llvm/llvm-project/commit/709112bce4424a5436f0bb699c62b3fbc837fbb6
DIFF: 
https://github.com/llvm/llvm-project/commit/709112bce4424a5436f0bb699c62b3fbc837fbb6.diff

LOG: [clang-tidy] false-positive for bugprone-redundant-branch-condition in 
case of passed-by-ref params

Inspired by discussion in https://reviews.llvm.org/D91037

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp

clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
index abb8e8be9b2f..2b0d9630527b 100644
--- a/clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
@@ -23,15 +23,21 @@ namespace bugprone {
 static const char CondVarStr[] = "cond_var";
 static const char OuterIfStr[] = "outer_if";
 static const char InnerIfStr[] = "inner_if";
+static const char OuterIfVar1Str[] = "outer_if_var1";
+static const char OuterIfVar2Str[] = "outer_if_var2";
+static const char InnerIfVar1Str[] = "inner_if_var1";
+static const char InnerIfVar2Str[] = "inner_if_var2";
 static const char FuncStr[] = "func";
 
-/// Returns whether `Var` is changed in `S` before `NextS`.
-static bool isChangedBefore(const Stmt *S, const Stmt *NextS,
+/// Returns whether `Var` is changed in range (`PrevS`..`NextS`).
+static bool isChangedBefore(const Stmt *S, const Stmt *NextS, const Stmt 
*PrevS,
 const VarDecl *Var, ASTContext *Context) {
   ExprMutationAnalyzer MutAn(*S, *Context);
   const auto  = Context->getSourceManager();
   const Stmt *MutS = MutAn.findMutation(Var);
   return MutS &&
+ SM.isBeforeInTranslationUnit(PrevS->getEndLoc(),
+  MutS->getBeginLoc()) &&
  SM.isBeforeInTranslationUnit(MutS->getEndLoc(), NextS->getBeginLoc());
 }
 
@@ -43,19 +49,22 @@ void 
RedundantBranchConditionCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
   ifStmt(
   hasCondition(ignoringParenImpCasts(anyOf(
-  declRefExpr(hasDeclaration(ImmutableVar)),
+  declRefExpr(hasDeclaration(ImmutableVar)).bind(OuterIfVar1Str),
   binaryOperator(hasOperatorName("&&"),
- 
hasEitherOperand(ignoringParenImpCasts(declRefExpr(
- hasDeclaration(ImmutableVar,
+ hasEitherOperand(ignoringParenImpCasts(
+ declRefExpr(hasDeclaration(ImmutableVar))
+ .bind(OuterIfVar2Str))),
   hasThen(hasDescendant(
   ifStmt(hasCondition(ignoringParenImpCasts(
- anyOf(declRefExpr(hasDeclaration(
-   varDecl(equalsBoundNode(CondVarStr,
+ anyOf(declRefExpr(hasDeclaration(varDecl(
+equalsBoundNode(CondVarStr
+.bind(InnerIfVar1Str),
binaryOperator(
hasAnyOperatorName("&&", "||"),
hasEitherOperand(ignoringParenImpCasts(
declRefExpr(hasDeclaration(varDecl(
-   
equalsBoundNode(CondVarStr)))
+ equalsBoundNode(CondVarStr
+ .bind(InnerIfVar2Str
   .bind(InnerIfStr))),
   forFunction(functionDecl().bind(FuncStr)))
   .bind(OuterIfStr),
@@ -69,15 +78,32 @@ void RedundantBranchConditionCheck::check(const 
MatchFinder::MatchResult 
   const auto *CondVar = Result.Nodes.getNodeAs(CondVarStr);
   const auto *Func = Result.Nodes.getNodeAs(FuncStr);
 
+  const DeclRefExpr *OuterIfVar, *InnerIfVar;
+  if (const auto *Inner = Result.Nodes.getNodeAs(InnerIfVar1Str))
+InnerIfVar = Inner;
+  else
+InnerIfVar = Result.Nodes.getNodeAs(InnerIfVar2Str);
+  if (const auto *Outer = Result.Nodes.getNodeAs(OuterIfVar1Str))
+OuterIfVar = Outer;
+  else
+OuterIfVar = Result.Nodes.getNodeAs(OuterIfVar2Str);
+
+  if (OuterIfVar && InnerIfVar) {
+if (isChangedBefore(OuterIf->getThen(), InnerIfVar, OuterIfVar, CondVar,
+Result.Context))
+  return;
+
+if (isChangedBefore(OuterIf->getCond(), InnerIfVar, OuterIfVar, CondVar,
+Result.Context))
+  return;
+  

[PATCH] D90157: [analyzer] Rework SValBuilder::evalCast function into maintainable and clear way

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@steakhal
Thank you for your invaluable analysis!

> What do you think @ASDenysPetrov ?

I think if there is any difference then I have to inspect the changes deeper. I 
tryed to make this changes in a way of not changing any behaviour.
Let me back to this patch a bit later. Currently have another important task.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90157

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

[PATCH] D93084: [VE] Optimize toolchain regression test

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/ve-toolchain.cpp:115
+// DEF-SAME: "-z" "max-page-size=0x400"
+// DEF-SAME: "crtbegin.o"
+// DEF-SAME: "-lc++" "-lc++abi" "-lunwind" "-lpthread" "-ldl"

You probably should add an empty file in `basic_ve_tree` so that users who 
don't have a system ve toolchain (most upstream developers) have 
`"...basic_ve_tree/...crtbegin.o"` instead of `"crtbegin.o"`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93084

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

[PATCH] D93084: [VE] Optimize toolchain regression test

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

LG.

I don't have a system VE toolchain and the tests still pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93084

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

[PATCH] D92041: [clangd] Add hover info for `this` expr

[xndcn via Phabricator via cfe-commits]

xndcn updated this revision to Diff 311256.
xndcn added a comment.

Thank you, it works like a charm!
For class withou template, `getHoverInfo(QualType ...)` will add namespace 
scope by default, so I have to add `SuppressScope` printpolicy here.


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

https://reviews.llvm.org/D92041

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2019,6 +2019,56 @@
 HI.NamespaceScope = "";
 HI.Definition = "@interface MYObject\n@end";
   }},
+  {
+  R"cpp(// this expr
+  // comment
+  namespace ns {
+class Foo {
+  Foo* bar() {
+return [[t^his]];
+  }
+};
+  }
+  )cpp",
+  [](HoverInfo ) { HI.Name = "Foo *"; }},
+  {
+  R"cpp(// this expr for template class
+  namespace ns {
+template 
+class Foo {
+  Foo* bar() const {
+return [[t^his]];
+  }
+};
+  }
+  )cpp",
+  [](HoverInfo ) { HI.Name = "const Foo *"; }},
+  {
+  R"cpp(// this expr for specialization class
+  namespace ns {
+template  class Foo {};
+template <>
+struct Foo {
+  Foo* bar() {
+return [[thi^s]];
+  }
+};
+  }
+  )cpp",
+  [](HoverInfo ) { HI.Name = "Foo *"; }},
+  {
+  R"cpp(// this expr for partial specialization struct
+  namespace ns {
+template  struct Foo {};
+template 
+struct Foo {
+  Foo* bar() const {
+return [[thi^s]];
+  }
+};
+  }
+  )cpp",
+  [](HoverInfo ) { HI.Name = "const Foo *"; }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -552,7 +552,8 @@
 
 /// Generate a \p Hover object given the type \p T.
 HoverInfo getHoverContents(QualType T, ASTContext ,
-   const SymbolIndex *Index) {
+   const SymbolIndex *Index,
+   bool SuppressScope = false) {
   HoverInfo HI;
 
   if (const auto *D = T->getAsTagDecl()) {
@@ -566,6 +567,7 @@
 // Builtin types
 auto Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
 Policy.SuppressTagKeyword = true;
+Policy.SuppressScope = SuppressScope;
 HI.Name = T.getAsString(Policy);
   }
   return HI;
@@ -628,15 +630,29 @@
   return llvm::StringLiteral("expression");
 }
 
-// Generates hover info for evaluatable expressions.
+// Generates hover info for `this` and evaluatable expressions.
 // FIXME: Support hover for literals (esp user-defined)
-llvm::Optional getHoverContents(const Expr *E, ParsedAST ) {
+llvm::Optional getHoverContents(const Expr *E, ParsedAST ,
+   const SymbolIndex *Index) {
   // There's not much value in hovering over "42" and getting a hover card
   // saying "42 is an int", similar for other literals.
   if (isLiteral(E))
 return llvm::None;
 
   HoverInfo HI;
+  // For `this` expr we currently generate hover with pointee type.
+  if (const CXXThisExpr *CTE = dyn_cast(E)) {
+QualType OriginThisType = CTE->getType()->getPointeeType();
+QualType ClassType = declaredType(OriginThisType->getAsTagDecl());
+// For partial specialization class, origin `this` pointee type will be
+// parsed as `InjectedClassNameType`, which will ouput template arguments
+// like "type-parameter-0-0". So we retrieve user written class type in this
+// case.
+QualType PrettyThisType = AST.getASTContext().getPointerType(
+QualType(ClassType.getTypePtr(), OriginThisType.getCVRQualifiers()));
+return getHoverContents(PrettyThisType, AST.getASTContext(), Index,
+/*SuppressScope=*/true);
+  }
   // For expressions we currently print the type and the value, iff it is
   // evaluatable.
   if (auto Val = printExprValue(E, AST.getASTContext())) {
@@ -861,7 +877,7 @@
   HI->Value = printExprValue(N, AST.getASTContext());
 maybeAddCalleeArgInfo(N, *HI, AST.getASTContext().getPrintingPolicy());
   } else if (const Expr *E = N->ASTNode.get()) {
-HI = getHoverContents(E, AST);
+HI = getHoverContents(E, AST, Index);
   }
   // FIXME: support hovers for other nodes?
   //  - 

[PATCH] D93023: Replace deprecated %T in 2 tests.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

My build directory is `/tmp/RelA`. After `sudo apt install 
gcc-10-riscv64-linux-gnu`,

  % /tmp/RelA/bin/llvm-lit -vv riscv32-toolchain-extra.c
  ...
  Exit Code: 1
  
  Command Output (stderr):
  --
  /home/ray/llvm/clang/test/Driver/riscv32-toolchain-extra.c:28:34: error: 
C-RV32-BAREMETAL-ILP32-NOGCC: expected string not found in input
  // C-RV32-BAREMETAL-ILP32-NOGCC: "-internal-isystem" 
"{{.*}}Output/testroot-riscv32-baremetal-nogcc/bin/../riscv32-unknown-elf/include"
   ^
  :1:1: note: scanning from here
  clang version 12.0.0
  ^
  :6:7: note: possible intended match here
   
"/tmp/RelA/tools/clang/test/Driver/Output/testroot-riscv32-baremetal-nogcc/bin/riscv32-unknown-elf-ld"
 "-m" "elf32lriscv" "crt0.o" 
"/usr/lib/gcc-cross/riscv64-linux-gnu/10/crtbegin.o" 
"-L/usr/lib/gcc-cross/riscv64-linux-gnu/10" 
"-L/usr/lib/gcc-cross/riscv64-linux-gnu/10/../../../../riscv64-linux-gnu/lib" 
"/tmp/riscv32-toolchain-extra-527779.o" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc" "/usr/lib/gcc-cross/riscv64-linux-gnu/10/crtend.o" "-o" 
"a.out"
^
  
  Input file: 
  Check file: /home/ray/llvm/clang/test/Driver/riscv32-toolchain-extra.c
  
  -dump-input=help explains the following input dump.
  
  Input was:
  <<
  1: clang version 12.0.0
  check:28'0 X~~~ error: no match found
  2: Target: riscv32-unknown-unknown-elf
  check:28'0 ~~~
  3: Thread model: posix
  check:28'0 ~~~
  4: InstalledDir: 
/tmp/RelA/tools/clang/test/Driver/Output/testroot-riscv32-baremetal-nogcc/bin
  check:28'0 
~~~
  5:  
"/tmp/RelA/tools/clang/test/Driver/Output/testroot-riscv32-baremetal-nogcc/bin/clang"
 "-cc1" "-triple" "riscv32-unknown-unknown-elf" "-emit-obj" "-mrelax-all" 
"--mrelax-relocations" "-disable-free" "-main-file-name" 
"riscv32-toolchain-extra.c" "-mrelocation-model" "static" "-mframe-pointer=all" 
"-fmath-errno" "-fno-rounding-math" "-mconstructor-aliases" "-nostdsysteminc" 
"-target-feature" "+m" "-target-feature" "+a" "-target-feature" "+c" 
"-target-feature" "+relax" "-target-feature" "-save-restore" "-target-abi" 
"ilp32" "-msmall-data-limit" "8" "-fno-split-dwarf-inlining" 
"-debugger-tuning=gdb" "-resource-dir" 
"/tmp/RelA/tools/clang/test/Driver/Output/testroot-riscv32-baremetal-nogcc/lib/clang/12.0.0"
 "-internal-isystem" 
"/usr/lib/gcc-cross/riscv64-linux-gnu/10/../../../../riscv64-linux-gnu/include" 
"-fdebug-compilation-dir" "/tmp/RelA/tools/clang/test/Driver" "-ferror-limit" 
"19" "-fno-signed-char" "-fgnuc-version=4.2.1" "-faddrsig" "-o" 
"/tmp/riscv32-toolchain-extra-527779.o" "-x" "c" 
"/home/ray/llvm/clang/test/Driver/riscv32-toolchain-extra.c"
  check:28'0 

  6:  
"/tmp/RelA/tools/clang/test/Driver/Output/testroot-riscv32-baremetal-nogcc/bin/riscv32-unknown-elf-ld"
 "-m" "elf32lriscv" "crt0.o" 
"/usr/lib/gcc-cross/riscv64-linux-gnu/10/crtbegin.o" 
"-L/usr/lib/gcc-cross/riscv64-linux-gnu/10" 
"-L/usr/lib/gcc-cross/riscv64-linux-gnu/10/../../../../riscv64-linux-gnu/lib" 
"/tmp/riscv32-toolchain-extra-527779.o" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc" "/usr/lib/gcc-cross/riscv64-linux-gnu/10/crtend.o" "-o" 
"a.out"
  check:28'0 

  check:28'1   

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

[Alan Phipps via Phabricator via cfe-commits]

alanphipps added a comment.

In D84467#2449045 , @vsk wrote:

> Unfortunately I don't think I'll be able to commit this on your behalf. Would 
> you be open to obtaining commit access from Chris and landing this directly? 
> I ask because, for a patch of this size, there's typically a fair amount of 
> post-commit feedback (primarily in the form of bot failure notifications, but 
> occasionally design feedback as well). It's common for tests to pass locally, 
> but not on some specific CI node, and it can take quite a bit of time to 
> resolve those issues.

Sure, that's reasonable, thanks.  I'll give that a shot.


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

https://reviews.llvm.org/D84467

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

[PATCH] D92977: [clangd] Improve hover and goToDefinition on auto and dectype

[Quentin Chateau via Phabricator via cfe-commits]

qchateau added a comment.

I've updated the patch with your latest comments.

Please commit this patch for me, using "quentin.chat...@gmail.com", I'll 
probably ask for commit access after a few successful reviews


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92977

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

[PATCH] D92977: [clangd] Improve hover and goToDefinition on auto and dectype

[Quentin Chateau via Phabricator via cfe-commits]

qchateau updated this revision to Diff 311247.
qchateau marked an inline comment as done.
qchateau added a comment.

- Reintroduce test with a FIXME comment
- locateSymbolForType returns a vector instead of an optional


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92977

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -624,6 +624,134 @@
 struct Fo^o {};
   )cpp",
 
+  R"cpp(// auto builtin type (not supported)
+^auto x = 42;
+  )cpp",
+
+  R"cpp(// auto on lambda
+auto x = [[[]]]{};
+^auto y = x;
+  )cpp",
+
+  R"cpp(// auto on struct
+namespace ns1 {
+struct [[S1]] {};
+} // namespace ns1
+
+^auto x = ns1::S1{};
+  )cpp",
+
+  R"cpp(// decltype on struct
+namespace ns1 {
+struct [[S1]] {};
+} // namespace ns1
+
+ns1::S1 i;
+^decltype(i) j;
+  )cpp",
+
+  R"cpp(// decltype(auto) on struct
+namespace ns1 {
+struct [[S1]] {};
+} // namespace ns1
+
+ns1::S1 i;
+ns1::S1& j = i;
+^decltype(auto) k = j;
+  )cpp",
+
+  R"cpp(// auto on template class
+template class [[Foo]] {};
+
+^auto x = Foo();
+  )cpp",
+
+  R"cpp(// auto on template class with forward declared class
+template class [[Foo]] {};
+class X;
+
+^auto x = Foo();
+  )cpp",
+
+  R"cpp(// auto on specialized template class
+template class Foo {};
+template<> class [[Foo]] {};
+
+^auto x = Foo();
+  )cpp",
+
+  R"cpp(// auto on initializer list.
+namespace std
+{
+  template
+  class [[initializer_list]] {};
+}
+
+^auto i = {1,2};
+  )cpp",
+
+  R"cpp(// auto function return with trailing type
+struct [[Bar]] {};
+^auto test() -> decltype(Bar()) {
+  return Bar();
+}
+  )cpp",
+
+  R"cpp(// decltype in trailing return type
+struct [[Bar]] {};
+auto test() -> ^decltype(Bar()) {
+  return Bar();
+}
+  )cpp",
+
+  R"cpp(// auto in function return
+struct [[Bar]] {};
+^auto test() {
+  return Bar();
+}
+  )cpp",
+
+  R"cpp(// auto& in function return
+struct [[Bar]] {};
+^auto& test() {
+  static Bar x;
+  return x;
+}
+  )cpp",
+
+  R"cpp(// auto* in function return
+struct [[Bar]] {};
+^auto* test() {
+  Bar* x;
+  return x;
+}
+  )cpp",
+
+  R"cpp(// const auto& in function return
+struct [[Bar]] {};
+const ^auto& test() {
+  static Bar x;
+  return x;
+}
+  )cpp",
+
+  R"cpp(// decltype(auto) in function return
+struct [[Bar]] {};
+^decltype(auto) test() {
+  return Bar();
+}
+  )cpp",
+
+  R"cpp(// decltype of function with trailing return type.
+struct [[Bar]] {};
+auto test() -> decltype(Bar()) {
+  return Bar();
+}
+void foo() {
+  ^decltype(test()) i = test();
+}
+  )cpp",
+
   R"cpp(// Override specifier jumps to overridden method
 class Y { virtual void $decl[[a]]() = 0; };
 class X : Y { void a() ^override {} };
Index: clang-tools-extra/clangd/unittests/ASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ASTTests.cpp
@@ -26,23 +26,168 @@
 namespace clangd {
 namespace {
 
-TEST(GetDeducedType, KwAutoExpansion) {
+TEST(GetDeducedType, KwAutoKwDecltypeExpansion) {
   struct Test {
 StringRef AnnotatedCode;
 const char *DeducedType;
   } Tests[] = {
   {"^auto i = 0;", "int"},
   {"^auto f(){ return 1;};", "int"},
+  {
+  R"cpp( // auto on struct in a namespace
+  namespace ns1 { struct S {}; }
+  ^auto v = ns1::S{};
+  )cpp",
+  "struct ns1::S",
+  },
+  {
+  R"cpp( // decltype on struct
+  namespace ns1 { struct S {}; }
+  ns1::S i;
+  ^decltype(i) j;
+  )cpp",
+  "ns1::S",
+  },
+  {
+  R"cpp(// decltype(auto) on struct&
+namespace ns1 {
+struct S {};
+} // namespace ns1
+
+ns1::S i;
+ns1::S& j = i;
+^decltype(auto) k = j;
+  )cpp",
+  "struct ns1::S &",
+  

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

In D84467#2448934 , @alanphipps wrote:

> In D84467#2423636 , @vsk wrote:
>
>> Thank you, lgtm!
>
> Thank you! I also would like to ask if you could commit it for me as I don't 
> have access for that.  Note that a handful of tests have binary files which 
> were not uploaded with the diff.  What do you advise I do to get those up 
> here so that I can commit it (unfortunately my system doesn't have arcanist 
> setup and that would take some time).  Thanks again for all of your help!

Unfortunately I don't think I'll be able to commit this on your behalf. Would 
you be open to obtaining commit access from Chris and landing this directly? I 
ask because, for a patch of this size, there's typically a fair amount of 
post-commit feedback (primarily in the form of bot failure notifications, but 
occasionally design feedback as well). It's common for tests to pass locally, 
but not on some specific CI node, and it can take quite a bit of time to 
resolve those issues.


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

https://reviews.llvm.org/D84467

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

[clang-tools-extra] 4d956af - Revert [clangd] Extract per-dir CDB cache to its own threadsafe class. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-12-11T17:35:50+01:00
New Revision: 4d956af594c5adc9d566d1846d86dd89c70c9c0b

URL: 
https://github.com/llvm/llvm-project/commit/4d956af594c5adc9d566d1846d86dd89c70c9c0b
DIFF: 
https://github.com/llvm/llvm-project/commit/4d956af594c5adc9d566d1846d86dd89c70c9c0b.diff

LOG: Revert [clangd] Extract per-dir CDB cache to its own threadsafe class. NFC

This reverts commit 8a4390dc4768fcd929a7231717980ccb28f124f7.

(The reland did not have the bugfix, just trying to get more details
from the buildbots)

Added: 


Modified: 
clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/clangd/GlobalCompilationDatabase.h

Removed: 




diff  --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp 
b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
index a50666ea8ccb..23e8c9fe716d 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -16,13 +16,11 @@
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
-#include 
 #include 
 #include 
 #include 
@@ -60,117 +58,10 @@ GlobalCompilationDatabase::getFallbackCommand(PathRef 
File) const {
   return Cmd;
 }
 
-// Loads and caches the CDB from a single directory.
-//
-// This class is threadsafe, which is to say we have independent locks for each
-// directory we're searching for a CDB.
-// Loading is deferred until first access.
-//
-// The DirectoryBasedCDB keeps a map from path => DirectoryCache.
-// Typical usage is to:
-//  - 1) determine all the paths that might be searched
-//  - 2) acquire the map lock and get-or-create all the DirectoryCache entries
-//  - 3) release the map lock and query the caches as desired
-//
-// FIXME: this should revalidate the cache sometimes
-// FIXME: IO should go through a VFS
-class DirectoryBasedGlobalCompilationDatabase::DirectoryCache {
-  // Absolute canonical path that we're the cache for. (Not case-folded).
-  const std::string Path;
-
-  // True if we've looked for a CDB here and found none.
-  // (This makes it possible for get() to return without taking a lock)
-  // FIXME: this should have an expiry time instead of lasting forever.
-  std::atomic FinalizedNoCDB = {false};
-
-  // Guards following cache state.
-  std::mutex Mu;
-  // Has cache been filled from disk? FIXME: this should be an expiry time.
-  bool CachePopulated = false;
-  // Whether a new CDB has been loaded but not broadcast yet.
-  bool NeedsBroadcast = false;
-  // Last loaded CDB, meaningful if CachePopulated is set.
-  // shared_ptr so we can overwrite this when callers are still using the CDB.
-  std::shared_ptr CDB;
-
-public:
-  DirectoryCache(llvm::StringRef Path) : Path(Path) {
-assert(llvm::sys::path::is_absolute(Path));
-  }
-
-  // Get the CDB associated with this directory.
-  // ShouldBroadcast:
-  //  - as input, signals whether the caller is willing to broadcast a
-  //newly-discovered CDB. (e.g. to trigger background indexing)
-  //  - as output, signals whether the caller should do so.
-  // (If a new CDB is discovered and ShouldBroadcast is false, we mark the
-  // CDB as needing broadcast, and broadcast it next time we can).
-  std::shared_ptr
-  get(bool ) {
-// Fast path for common case without taking lock.
-if (FinalizedNoCDB.load()) {
-  ShouldBroadcast = false;
-  return nullptr;
-}
-std::lock_guard Lock(Mu);
-auto RequestBroadcast = llvm::make_scope_exit([&, OldCDB(CDB.get())] {
-  // If we loaded a new CDB, it should be broadcast at some point.
-  if (CDB != nullptr && CDB.get() != OldCDB)
-NeedsBroadcast = true;
-  else if (CDB == nullptr) // nothing to broadcast anymore!
-NeedsBroadcast = false;
-  // If we have something to broadcast, then do so iff allowed.
-  if (!ShouldBroadcast)
-return;
-  ShouldBroadcast = NeedsBroadcast;
-  NeedsBroadcast = false;
-});
-
-// For now, we never actually attempt to revalidate a populated cache.
-if (CachePopulated)
-  return CDB;
-assert(CDB == nullptr);
-
-load();
-CachePopulated = true;
-
-if (!CDB)
-  FinalizedNoCDB.store(true);
-return CDB;
-  }
-
-  llvm::StringRef path() const { return Path; }
-
-private:
-  // Updates `CDB` from disk state.
-  void load() {
-std::string Error; // ignored, because it's often "didn't find anything".
-CDB = tooling::CompilationDatabase::loadFromDirectory(Path, Error);
-if (!CDB) {
-  // Fallback: check for $src/build, the conventional CMake build root.
-  // Probe existence first to avoid each plugin doing IO if it doesn't
-  // 

[PATCH] D92761: [clang][AArch64][SVE] Avoid going through memory for VLAT <-> VLST casts

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks resigned from this revision.
aeubanks added a comment.

I'm unfamiliar with this code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92761

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

[clang-tools-extra] 8a4390d - Reland [clangd] Extract per-dir CDB cache to its own threadsafe class. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-12-11T17:34:53+01:00
New Revision: 8a4390dc4768fcd929a7231717980ccb28f124f7

URL: 
https://github.com/llvm/llvm-project/commit/8a4390dc4768fcd929a7231717980ccb28f124f7
DIFF: 
https://github.com/llvm/llvm-project/commit/8a4390dc4768fcd929a7231717980ccb28f124f7.diff

LOG: Reland [clangd] Extract per-dir CDB cache to its own threadsafe class. NFC

This reverts commit de4f5519015cc97f28718d90cc6dac73c0a15161.

More debug output to try to pin down an impossible condition.

Added: 


Modified: 
clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/clangd/GlobalCompilationDatabase.h

Removed: 




diff  --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp 
b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
index 23e8c9fe716d..a50666ea8ccb 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -16,11 +16,13 @@
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include 
 #include 
 #include 
 #include 
@@ -58,10 +60,117 @@ GlobalCompilationDatabase::getFallbackCommand(PathRef 
File) const {
   return Cmd;
 }
 
+// Loads and caches the CDB from a single directory.
+//
+// This class is threadsafe, which is to say we have independent locks for each
+// directory we're searching for a CDB.
+// Loading is deferred until first access.
+//
+// The DirectoryBasedCDB keeps a map from path => DirectoryCache.
+// Typical usage is to:
+//  - 1) determine all the paths that might be searched
+//  - 2) acquire the map lock and get-or-create all the DirectoryCache entries
+//  - 3) release the map lock and query the caches as desired
+//
+// FIXME: this should revalidate the cache sometimes
+// FIXME: IO should go through a VFS
+class DirectoryBasedGlobalCompilationDatabase::DirectoryCache {
+  // Absolute canonical path that we're the cache for. (Not case-folded).
+  const std::string Path;
+
+  // True if we've looked for a CDB here and found none.
+  // (This makes it possible for get() to return without taking a lock)
+  // FIXME: this should have an expiry time instead of lasting forever.
+  std::atomic FinalizedNoCDB = {false};
+
+  // Guards following cache state.
+  std::mutex Mu;
+  // Has cache been filled from disk? FIXME: this should be an expiry time.
+  bool CachePopulated = false;
+  // Whether a new CDB has been loaded but not broadcast yet.
+  bool NeedsBroadcast = false;
+  // Last loaded CDB, meaningful if CachePopulated is set.
+  // shared_ptr so we can overwrite this when callers are still using the CDB.
+  std::shared_ptr CDB;
+
+public:
+  DirectoryCache(llvm::StringRef Path) : Path(Path) {
+assert(llvm::sys::path::is_absolute(Path));
+  }
+
+  // Get the CDB associated with this directory.
+  // ShouldBroadcast:
+  //  - as input, signals whether the caller is willing to broadcast a
+  //newly-discovered CDB. (e.g. to trigger background indexing)
+  //  - as output, signals whether the caller should do so.
+  // (If a new CDB is discovered and ShouldBroadcast is false, we mark the
+  // CDB as needing broadcast, and broadcast it next time we can).
+  std::shared_ptr
+  get(bool ) {
+// Fast path for common case without taking lock.
+if (FinalizedNoCDB.load()) {
+  ShouldBroadcast = false;
+  return nullptr;
+}
+std::lock_guard Lock(Mu);
+auto RequestBroadcast = llvm::make_scope_exit([&, OldCDB(CDB.get())] {
+  // If we loaded a new CDB, it should be broadcast at some point.
+  if (CDB != nullptr && CDB.get() != OldCDB)
+NeedsBroadcast = true;
+  else if (CDB == nullptr) // nothing to broadcast anymore!
+NeedsBroadcast = false;
+  // If we have something to broadcast, then do so iff allowed.
+  if (!ShouldBroadcast)
+return;
+  ShouldBroadcast = NeedsBroadcast;
+  NeedsBroadcast = false;
+});
+
+// For now, we never actually attempt to revalidate a populated cache.
+if (CachePopulated)
+  return CDB;
+assert(CDB == nullptr);
+
+load();
+CachePopulated = true;
+
+if (!CDB)
+  FinalizedNoCDB.store(true);
+return CDB;
+  }
+
+  llvm::StringRef path() const { return Path; }
+
+private:
+  // Updates `CDB` from disk state.
+  void load() {
+std::string Error; // ignored, because it's often "didn't find anything".
+CDB = tooling::CompilationDatabase::loadFromDirectory(Path, Error);
+if (!CDB) {
+  // Fallback: check for $src/build, the conventional CMake build root.
+  // Probe existence first to avoid each plugin doing IO if it doesn't
+  // exist.
+  

[PATCH] D92854: [flang][driver] Add support for `-fsyntax-only`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski updated this revision to Diff 311231.
awarzynski added a comment.

Fine-tune the output for consistency with `flang`/`f18`

This is just a minor change to make sure that:

- `flang-new -fc1 -fsyntax-only`, and
- `f18 -fparse-only`

are as similar as possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92854

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/CompilerInstance.h
  flang/include/flang/Frontend/FrontendActions.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Flang-Driver/syntax-only.f90
  flang/unittests/Frontend/PrintPreprocessedTest.cpp

Index: flang/unittests/Frontend/PrintPreprocessedTest.cpp
===
--- flang/unittests/Frontend/PrintPreprocessedTest.cpp
+++ flang/unittests/Frontend/PrintPreprocessedTest.cpp
@@ -76,4 +76,60 @@
   llvm::sys::fs::remove(inputFile);
   compInst.ClearOutputFiles(/*EraseFiles=*/true);
 }
+
+TEST(FrontendAction, ParseSyntaxOnly) {
+  std::string inputFile = "test-file.f";
+  std::error_code ec;
+
+  // 1. Create the input file for the file manager
+  // AllSources (which is used to manage files inside every compiler instance),
+  // works with paths. This means that it requires a physical file. Create one.
+  std::unique_ptr os{
+  new llvm::raw_fd_ostream(inputFile, ec, llvm::sys::fs::OF_None)};
+  if (ec)
+FAIL() << "Fail to create the file need by the test";
+
+  // Populate the input file with the pre-defined input and flush it.
+  *(os) << "! if_stmt.f90:\n"
+<< "IF (A > 0.0) IF (B < 0.0) A = LOG (A)\n"
+<< "END";
+  os.reset();
+
+  // Get the path of the input file
+  llvm::SmallString<64> cwd;
+  if (std::error_code ec = llvm::sys::fs::current_path(cwd))
+FAIL() << "Failed to obtain the current working directory";
+  std::string testFilePath(cwd.c_str());
+  testFilePath += "/" + inputFile;
+
+  // 2. Prepare the compiler (CompilerInvocation + CompilerInstance)
+  CompilerInstance compInst;
+  compInst.CreateDiagnostics();
+  auto invocation = std::make_shared();
+  invocation->frontendOpts().programAction_ = ParseSyntaxOnly;
+
+  compInst.set_invocation(std::move(invocation));
+  compInst.frontendOpts().inputs_.push_back(
+  FrontendInputFile(testFilePath, Language::Fortran));
+
+  // 3. Set-up the output stream for the semantic diagnostics.
+  llvm::SmallVector outputDiagBuffer;
+  std::unique_ptr outputStream(
+  new llvm::raw_svector_ostream(outputDiagBuffer));
+  compInst.set_semaOutputStream(std::move(outputStream));
+
+  // 4. Execute the ParseSyntaxOnly action
+  bool success = ExecuteCompilerInvocation();
+
+  // 5. Validate the expected output
+  EXPECT_FALSE(success);
+  EXPECT_TRUE(!outputDiagBuffer.empty());
+  EXPECT_TRUE(
+  llvm::StringRef(outputDiagBuffer.data())
+  .startswith(
+  ":2:14: error: IF statement is not allowed in IF statement\n"));
+
+  // 6. Clear the input files.
+  llvm::sys::fs::remove(inputFile);
+}
 } // namespace
Index: flang/test/Flang-Driver/syntax-only.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/syntax-only.f90
@@ -0,0 +1,9 @@
+! RUN: not %flang-new -fc1 -fsyntax-only %s 2>&1 | FileCheck %s
+! RUN: not %f18 -fparse-only %s 2>&1 | FileCheck %s
+
+! REQUIRES: new-flang-driver
+
+! CHECK: IF statement is not allowed in IF statement
+! CHECK: semantic errors in {{.*}}syntax-only.f90
+IF (A > 0.0) IF (B < 0.0) A = LOG (A)
+END
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -32,6 +32,9 @@
   case PrintPreprocessedInput:
 return std::make_unique();
 break;
+  case ParseSyntaxOnly:
+return std::make_unique();
+break;
   default:
 break;
 // TODO:
Index: flang/lib/Frontend/FrontendActions.cpp
===
--- flang/lib/Frontend/FrontendActions.cpp
+++ flang/lib/Frontend/FrontendActions.cpp
@@ -11,6 +11,7 @@
 #include "flang/Parser/parsing.h"
 #include "flang/Parser/provenance.h"
 #include "flang/Parser/source.h"
+#include "flang/Semantics/semantics.h"
 
 using namespace Fortran::frontend;
 
@@ -68,3 +69,34 @@
 return;
   }
 }
+
+void ParseSyntaxOnlyAction::ExecuteAction() {
+  CompilerInstance  = this->instance();
+
+  // TODO: These should be specifiable by users. For now just use the defaults.
+  common::LanguageFeatureControl features;
+  Fortran::common::IntrinsicTypeDefaultKinds defaultKinds;
+
+  // 

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

[Valeriy Savchenko via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/Attr.td:1774
+  let LangOpts = [ObjC];
+  let Documentation = [Undocumented];
+}

aaron.ballman wrote:
> No new undocumented attributes, please.
Sure, will do!



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3623
+static void handleCalledOnceAttr(Sema , Decl *D, const ParsedAttr ) {
+  if (D->isInvalidDecl())
+return;

aaron.ballman wrote:
> I don't think there's a need for this check -- attaching the attribute to an 
> invalid declaration doesn't hurt anything downstream (I believe), but failing 
> to attach the attribute harms AST fidelity for the folks doing work on 
> retaining erroneous AST nodes.
Sure, simply did it like it's done for some other attribute for parameters.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3630
+  if (!isFunctionLike(*T)) {
+S.Diag(AL.getLoc(), diag::err_called_once_attribute_wrong_type);
+return;

aaron.ballman wrote:
> Because there can be multiple parameters in the declaration, passing the 
> range to the specific problematic parameter is helpful.
I was just thinking that because attribute applies to each parameter separately 
and we already show this error at the location of the attribute, it is clear 
which parameter is implied.



Comment at: clang/test/SemaObjC/warn-called-once.m:887
+}
+@end

aaron.ballman wrote:
> Can you also add a test that shows this works with lambda or block parameters 
> that contain the attribute? e.g.,
> ```
> [](void (*fp CALLED_ONCE)()) { ... }(some_fp);
> ```
> Also, you should add some tests that the attribute diagnoses when applied to 
> something other than a parameter, is given an argument, and a test that the 
> attribute is rejected unless in ObjC.
There is a test here called `block_with_called_once` that covers that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92039

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

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

[Alan Phipps via Phabricator via cfe-commits]

alanphipps added a comment.

In D84467#2423636 , @vsk wrote:

> Thank you, lgtm!

Thank you! I also would like to ask if you could commit it for me as I don't 
have access for that.  Note that a handful of tests have binary files which 
were not uploaded with the diff.  What do you advise I do to get those up here 
so that I can commit it (unfortunately my system doesn't have arcanist setup 
and that would take some time).  Thanks again for all of your help!


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

https://reviews.llvm.org/D84467

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

[PATCH] D92977: [clangd] Improve hover and goToDefinition on auto and dectype

[Sam McCall via Phabricator via cfe-commits]

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

Looks good, thanks again!
If you don't yet have LLVM commit access, I can commit this for you, let me 
know which email address to associate it with.
(https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access describes 
how you can get commit access if you're contributing regularly)




Comment at: clang-tools-extra/clangd/XRefs.cpp:442
+  const auto  = AST.getASTContext();
+  const NamedDecl *D = getPreferredDecl(Decls.front());
+

nit: we do often just use the first element (there's almost always just one), 
but here we can actually return multiple LocatedSymbols (client shows some sort 
of selector widget for them) so I'd suggest we loop and return all



Comment at: clang-tools-extra/clangd/XRefs.cpp:689
+const NamedDecl *D = Deduced->getTypePtr()->getAsTagDecl();
+if (!D)
+  continue;

qchateau wrote:
> sammccall wrote:
> > there are many other types we could resolve to a decl: TypedefType, 
> > TemplateTypeParmtype, ...
> > If we're only going to handle tag types, it deserves a FIXME comment.
> > 
> > But we do have  a helper for this. Can you try calling 
> > `targetDecl(DynTypedNode::create(Deduced), DeclRelation::TemplatePattern | 
> > DeclRelation::Alias)` instead of `Deduced->getTypePtr()->getAsTagDecl()`?
> > 
> > That should handle more cases, at a minimum, this case sholud pass:
> > ```
> > using [[T]] = int;
> > T x;
> > ^auto y = x;
> > ```
> > 
> > (I see you have a test case that aliases backed by tag types are resolved 
> > to the underlying tag decl, this change would offer the alias instead, 
> > which is more consistent with our current go-to-definition. You could also 
> > offer both by passing `DeclRelation::TemplatePattern | DeclRelation::Alias 
> > | DeclRelation::Underlying`... but I think we should value consistency here)
> `locateSymbolForType` uses `targetDecl`. Thanks for the tip, that's exactly 
> what I was looking for.
> 
> I also agree with you that go-to-definition should go to the alias instead of 
> the underlying type for consistency, but using 
> `targetDecl(DynTypedNode::create(Deduced), DeclRelation::TemplatePattern | 
> DeclRelation::Alias)` is not enough to resolve this consistency issue: 
> `getDeducedType` already returns the underlying type. My current knowledge of 
> the clangd code is nou sufficient to understand why, but the root of the 
> problem seem to lie in the `DeducedTypeVisitor` class.
> 
> I removed the test that tested the undesirable behavior, should I open a bug 
> report for `getDeducedType` that should not "go through" aliases ? In which 
> case, what's the right place for that ? Github ?
Great!
Marginally better than removing a test is keeping it with a comment `// FIXME: 
it'd be nice if this resolved to the alias instead` or something like that

Github is the right place to file a bug, though I don't think this is really a 
big deal, so sometimes we just leave the comment.

(I'm not sure offhand if the AST actually preserves the sugar type or just the 
underlying one)



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:669
+
+  R"cpp(// auto on template class with forward declared class
+template class [[Foo]] {};

qchateau wrote:
> sammccall wrote:
> > There's a scalability problem with testing every corner of the C++ language 
> > (there are lots!) with every feature. Generally I prefer to cover some 
> > important/weird/bugfix cases here, but to cover as much as possible with 
> > unittests of the underlying functions.
> > 
> > In this case, that would mean
> >  - moving tests that are about `auto` in different contexts to ASTTests.cpp 
> > (current test coverage there is poor).
> >  - (assuming we can use `targetDecl()`) moving tests that are about which 
> > decl a type should resolve to to `FindTargetTests.cpp` - but current 
> > coverage there is pretty good so we can probably just drop many of them.
> I added the relevant tests to `ASTTests.cpp` but I did not remove the tests 
> here for the moment. I've always had the habit to keep tests that are already 
> written, even if redundant.
> 
> If you'd like me to remove some of them, I'd say the only tests that are not 
> redundant with the ones in `ASTTests.cpp` are the ones that test template 
> specializations. Shoud I keep only these ?
Keeping them around is fine too, we're not strict about this.

Thanks for adding all the tests for deduced types!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92977

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

[PATCH] D84188: Port FileSystem options to new option parsing system

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 commandeered this revision.
jansvoboda11 added a reviewer: dang.
jansvoboda11 added a comment.

Taking over this patch as Daniel is no longer involved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84188

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

[PATCH] D92720: [HIP] unbundle bundled preprocessor output

[Yaxun Liu via Phabricator via cfe-commits]

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

In D92720#2437621 , @tra wrote:

> `-E` by default prints preprocessed output to stdout. CUDA will print 
> preprocessed output from all subcompilations. What does HIP do in this case? 
> Printing out the bundle is probably not what the user will expect.
> IMO preprocessed output is frequently used as a debugging tool, so it's 
> important for users to be able to read it. Bundled output is rather 
> cumbersome to deal with. It's possible to manually unbundle it, but the tool 
> is not documented well and it's not particularly suitable for human use.
>
> I think we should preserve the long-established meaning of `-E` and keep its 
> output as plain text.

Output of `-E` for HIP combined host/device compilation is a plain text. It has 
C++ comments inserted between preprocessor outputs for host and different GPU 
arch's. The C++ comments follow the format of clang-offload-bundler bundled 
text files therefore clang-offload-bundler is able to unbundle it.




Comment at: clang/lib/Driver/Driver.cpp:2523-2524
 // this input.
-if (IA->getType() != types::TY_CUDA &&
-IA->getType() != types::TY_HIP) {
+if (IA->getType() != types::TY_CUDA && IA->getType() != types::TY_HIP 
&&
+IA->getType() != types::TY_PP_HIP) {
   // The builder will ignore this input.

tra wrote:
> Nit: too many negations in the condition. `if (! (IA->getType() == 
> types::TY_CUDA || IA->getType() == types::TY_HIP  || IA->getType() == 
> types::TY_PP_HIP))` would be easier to understand, IMO.
> 
> 
done


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

https://reviews.llvm.org/D92720

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

[PATCH] D84018: Port Preprocessor and PreprocessorOutput option flags to new option parsing system

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a reviewer: dexonsmith.
jansvoboda11 added a comment.

Ready for a review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84018

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

[PATCH] D84018: Port Preprocessor and PreprocessorOutput option flags to new option parsing system

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 311226.
jansvoboda11 added a comment.

Rebase, undo unecessary move of options


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84018

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3254,16 +3254,11 @@
   Opts.ImplicitPCHInclude = std::string(Args.getLastArgValue(OPT_include_pch));
   Opts.PCHWithHdrStop = Args.hasArg(OPT_pch_through_hdrstop_create) ||
 Args.hasArg(OPT_pch_through_hdrstop_use);
-  Opts.PCHWithHdrStopCreate = Args.hasArg(OPT_pch_through_hdrstop_create);
   Opts.PCHThroughHeader =
   std::string(Args.getLastArgValue(OPT_pch_through_header_EQ));
-  Opts.UsePredefines = !Args.hasArg(OPT_undef);
-  Opts.DetailedRecord = Args.hasArg(OPT_detailed_preprocessing_record);
-  Opts.DisablePCHValidation = Args.hasArg(OPT_fno_validate_pch);
   Opts.AllowPCHWithCompilerErrors =
   Args.hasArg(OPT_fallow_pch_with_errors, OPT_fallow_pcm_with_errors);
 
-  Opts.DumpDeserializedPCHDecls = Args.hasArg(OPT_dump_deserialized_pch_decls);
   for (const auto *A : Args.filtered(OPT_error_on_deserialized_pch_decl))
 Opts.DeserializedPCHDeclsToErrorOn.insert(A->getValue());
 
@@ -3346,9 +3341,6 @@
   // "editor placeholder in source file" error in PP only mode.
   if (isStrictlyPreprocessorAction(Action))
 Opts.LexEditorPlaceholders = false;
-
-  Opts.SetUpStaticAnalyzer = Args.hasArg(OPT_setup_static_analyzer);
-  Opts.DisablePragmaDebugCrash = Args.hasArg(OPT_disable_pragma_debug_crash);
 }
 
 static void ParsePreprocessorOutputArgs(PreprocessorOutputOptions ,
@@ -3359,14 +3351,7 @@
   else
 Opts.ShowCPP = 0;
 
-  Opts.ShowComments = Args.hasArg(OPT_C);
-  Opts.ShowLineMarkers = !Args.hasArg(OPT_P);
-  Opts.ShowMacroComments = Args.hasArg(OPT_CC);
   Opts.ShowMacros = Args.hasArg(OPT_dM) || Args.hasArg(OPT_dD);
-  Opts.ShowIncludeDirectives = Args.hasArg(OPT_dI);
-  Opts.RewriteIncludes = Args.hasArg(OPT_frewrite_includes);
-  Opts.RewriteImports = Args.hasArg(OPT_frewrite_imports);
-  Opts.UseLineDirectives = Args.hasArg(OPT_fuse_line_directives);
 }
 
 static void ParseTargetArgs(TargetOptions , ArgList ,
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -623,9 +623,11 @@
 def B : JoinedOrSeparate<["-"], "B">, MetaVarName<"">,
 HelpText<"Add  to search path for binaries and object files used implicitly">;
 def CC : Flag<["-"], "CC">, Flags<[CC1Option]>, Group,
-HelpText<"Include comments from within macros in preprocessed output">;
+HelpText<"Include comments from within macros in preprocessed output">,
+MarshallingInfoFlag<"PreprocessorOutputOpts.ShowMacroComments">;
 def C : Flag<["-"], "C">, Flags<[CC1Option]>, Group,
-HelpText<"Include comments in preprocessed output">;
+HelpText<"Include comments in preprocessed output">,
+MarshallingInfoFlag<"PreprocessorOutputOpts.ShowComments">;
 def D : JoinedOrSeparate<["-"], "D">, Group,
 Flags<[CC1Option]>, MetaVarName<"=">,
 HelpText<"Define  to  (or 1 if  omitted)">;
@@ -691,7 +693,8 @@
 def O_flag : Flag<["-"], "O">, Flags<[CC1Option]>, Alias, AliasArgs<["1"]>;
 def Ofast : Joined<["-"], "Ofast">, Group, Flags<[CC1Option]>;
 def P : Flag<["-"], "P">, Flags<[CC1Option]>, Group,
-  HelpText<"Disable linemarker output in -E mode">;
+  HelpText<"Disable linemarker output in -E mode">,
+  MarshallingInfoFlag<"PreprocessorOutputOpts.ShowLineMarkers", "true">, IsNegative;
 def Qy : Flag<["-"], "Qy">, Flags<[CC1Option]>,
   HelpText<"Emit metadata containing compiler name and version">;
 def Qn : Flag<["-"], "Qn">, Flags<[CC1Option]>,
@@ -951,7 +954,8 @@
 def dD : Flag<["-"], "dD">, Group, Flags<[CC1Option]>,
   HelpText<"Print macro definitions in -E mode in addition to normal output">;
 def dI : Flag<["-"], "dI">, Group, Flags<[CC1Option]>,
-  HelpText<"Print include directives in -E mode in addition to normal output">;
+  HelpText<"Print include directives in -E mode in addition to normal output">,
+  MarshallingInfoFlag<"PreprocessorOutputOpts.ShowIncludeDirectives">;
 def dM : Flag<["-"], "dM">, Group, Flags<[CC1Option]>,
   HelpText<"Print macro definitions in -E mode instead of normal output">;
 def dead__strip : Flag<["-"], "dead_strip">;
@@ -1574,8 +1578,12 @@
 def ffor_scope : Flag<["-"], "ffor-scope">, Group;
 def fno_for_scope : Flag<["-"], "fno-for-scope">, Group;
 
-defm rewrite_imports : OptInFFlag<"rewrite-imports", "">;
-defm rewrite_includes : OptInFFlag<"rewrite-includes", "">;
+defm rewrite_imports : BoolFOption<"rewrite-imports",
+  

[PATCH] D92977: [clangd] Improve hover and goToDefinition on auto and dectype

[Quentin Chateau via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/XRefs.cpp:689
+const NamedDecl *D = Deduced->getTypePtr()->getAsTagDecl();
+if (!D)
+  continue;

sammccall wrote:
> there are many other types we could resolve to a decl: TypedefType, 
> TemplateTypeParmtype, ...
> If we're only going to handle tag types, it deserves a FIXME comment.
> 
> But we do have  a helper for this. Can you try calling 
> `targetDecl(DynTypedNode::create(Deduced), DeclRelation::TemplatePattern | 
> DeclRelation::Alias)` instead of `Deduced->getTypePtr()->getAsTagDecl()`?
> 
> That should handle more cases, at a minimum, this case sholud pass:
> ```
> using [[T]] = int;
> T x;
> ^auto y = x;
> ```
> 
> (I see you have a test case that aliases backed by tag types are resolved to 
> the underlying tag decl, this change would offer the alias instead, which is 
> more consistent with our current go-to-definition. You could also offer both 
> by passing `DeclRelation::TemplatePattern | DeclRelation::Alias | 
> DeclRelation::Underlying`... but I think we should value consistency here)
`locateSymbolForType` uses `targetDecl`. Thanks for the tip, that's exactly 
what I was looking for.

I also agree with you that go-to-definition should go to the alias instead of 
the underlying type for consistency, but using 
`targetDecl(DynTypedNode::create(Deduced), DeclRelation::TemplatePattern | 
DeclRelation::Alias)` is not enough to resolve this consistency issue: 
`getDeducedType` already returns the underlying type. My current knowledge of 
the clangd code is nou sufficient to understand why, but the root of the 
problem seem to lie in the `DeducedTypeVisitor` class.

I removed the test that tested the undesirable behavior, should I open a bug 
report for `getDeducedType` that should not "go through" aliases ? In which 
case, what's the right place for that ? Github ?



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:669
+
+  R"cpp(// auto on template class with forward declared class
+template class [[Foo]] {};

sammccall wrote:
> There's a scalability problem with testing every corner of the C++ language 
> (there are lots!) with every feature. Generally I prefer to cover some 
> important/weird/bugfix cases here, but to cover as much as possible with 
> unittests of the underlying functions.
> 
> In this case, that would mean
>  - moving tests that are about `auto` in different contexts to ASTTests.cpp 
> (current test coverage there is poor).
>  - (assuming we can use `targetDecl()`) moving tests that are about which 
> decl a type should resolve to to `FindTargetTests.cpp` - but current coverage 
> there is pretty good so we can probably just drop many of them.
I added the relevant tests to `ASTTests.cpp` but I did not remove the tests 
here for the moment. I've always had the habit to keep tests that are already 
written, even if redundant.

If you'd like me to remove some of them, I'd say the only tests that are not 
redundant with the ones in `ASTTests.cpp` are the ones that test template 
specializations. Shoud I keep only these ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92977

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

[PATCH] D92720: [HIP] unbundle bundled preprocessor output

[Yaxun Liu via Phabricator via cfe-commits]

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

revised by Artem's comments


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

https://reviews.llvm.org/D92720

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/hip-unbundle-preproc.hip


Index: clang/test/Driver/hip-unbundle-preproc.hip
===
--- /dev/null
+++ clang/test/Driver/hip-unbundle-preproc.hip
@@ -0,0 +1,25 @@
+// REQUIRES: clang-driver, amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu \
+// RUN:   --offload-arch=gfx803 -nogpulib \
+// RUN:   -x hip-cpp-output %s 2>&1 | FileCheck %s
+
+// CHECK: {{".*clang-offload-bundler.*"}} 
{{.*}}"-outputs=[[HOST_PP:.*cui]],[[DEV_PP:.*cui]]" "-unbundle"
+// CHECK: {{".*clang.*"}} "-cc1" {{.*}}"-target-cpu" "gfx803" {{.*}}"-o" 
"[[DEV_O:.*o]]" {{.*}}"[[DEV_PP]]"
+// CHECK: {{".*lld.*"}} {{.*}}"-o" "[[DEV_ISA:.*]]" "[[DEV_O]]"
+// CHECK: {{".*clang-offload-bundler.*"}} {{.*}}"-inputs={{.*}},[[DEV_ISA]]" 
"-outputs=[[FATBIN:.*]]"
+// CHECK: {{".*clang.*"}} {{.*}}"-triple" "x86_64-unknown-linux-gnu"{{.*}} 
"-fcuda-include-gpubinary" "[[FATBIN]]" {{.*}}"-o" "[[HOST_O:.*o]]" 
{{.*}}"[[HOST_PP]]"
+// CHECK: {{".*ld.*"}} {{.*}}"[[HOST_O]]"
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu \
+// RUN:   --offload-arch=gfx803 -nogpulib -fgpu-rdc \
+// RUN:   -x hip-cpp-output %s 2>&1 | FileCheck -check-prefix=RDC %s
+
+// RDC: {{".*clang-offload-bundler.*"}} 
{{.*}}"-outputs=[[HOST_PP:.*cui]],[[DEV_PP:.*cui]]" "-unbundle"
+// RDC: {{".*clang.*"}} {{.*}}"-triple" "x86_64-unknown-linux-gnu"{{.*}} "-o" 
"[[HOST_O:.*o]]" {{.*}}"[[HOST_PP]]"
+// RDC: {{".*clang-offload-bundler.*"}} 
{{.*}}"-outputs=[[HOST_PP:.*cui]],[[DEV_PP:.*cui]]" "-unbundle"
+// RDC: {{".*clang.*"}} "-cc1" {{.*}}"-target-cpu" "gfx803" {{.*}}"-o" 
"[[DEV_BC:.*bc]]" {{.*}}"[[DEV_PP]]"
+// RDC: {{".*lld.*"}} {{.*}}"-o" "[[DEV_ISA:.*]]" "[[DEV_BC]]"
+// RDC: {{".*clang-offload-bundler.*"}} {{.*}}"-inputs={{.*}},[[DEV_ISA]]" 
"-outputs=[[FATBIN:.*]]"
+// RDC: {{".*llvm-mc.*"}} "-o" "[[FATBIN_O:.*o]]"
+// RDC: {{".*ld.*"}} {{.*}}"[[HOST_O]]" "[[FATBIN_O]]"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2520,8 +2520,9 @@
 
 // If the host input is not CUDA or HIP, we don't need to bother about
 // this input.
-if (IA->getType() != types::TY_CUDA &&
-IA->getType() != types::TY_HIP) {
+if (!(IA->getType() == types::TY_CUDA ||
+  IA->getType() == types::TY_HIP ||
+  IA->getType() == types::TY_PP_HIP)) {
   // The builder will ignore this input.
   IsActive = false;
   return ABRT_Inactive;
@@ -2550,7 +2551,7 @@
 
 // If -fgpu-rdc is disabled, should not unbundle since there is no
 // device code to link.
-if (!Relocatable)
+if (UA->getType() == types::TY_Object && !Relocatable)
   return ABRT_Inactive;
 
 CudaDeviceActions.clear();
@@ -3450,7 +3451,8 @@
 // the input is not a bundle.
 if (CanUseBundler && isa(HostAction) &&
 InputArg->getOption().getKind() == llvm::opt::Option::InputClass &&
-!types::isSrcFile(HostAction->getType())) {
+(!types::isSrcFile(HostAction->getType()) ||
+ HostAction->getType() == types::TY_PP_HIP)) {
   auto UnbundlingHostAction =
   C.MakeAction(HostAction);
   UnbundlingHostAction->registerDependentActionInfo(


Index: clang/test/Driver/hip-unbundle-preproc.hip
===
--- /dev/null
+++ clang/test/Driver/hip-unbundle-preproc.hip
@@ -0,0 +1,25 @@
+// REQUIRES: clang-driver, amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu \
+// RUN:   --offload-arch=gfx803 -nogpulib \
+// RUN:   -x hip-cpp-output %s 2>&1 | FileCheck %s
+
+// CHECK: {{".*clang-offload-bundler.*"}} {{.*}}"-outputs=[[HOST_PP:.*cui]],[[DEV_PP:.*cui]]" "-unbundle"
+// CHECK: {{".*clang.*"}} "-cc1" {{.*}}"-target-cpu" "gfx803" {{.*}}"-o" "[[DEV_O:.*o]]" {{.*}}"[[DEV_PP]]"
+// CHECK: {{".*lld.*"}} {{.*}}"-o" "[[DEV_ISA:.*]]" "[[DEV_O]]"
+// CHECK: {{".*clang-offload-bundler.*"}} {{.*}}"-inputs={{.*}},[[DEV_ISA]]" "-outputs=[[FATBIN:.*]]"
+// CHECK: {{".*clang.*"}} {{.*}}"-triple" "x86_64-unknown-linux-gnu"{{.*}} "-fcuda-include-gpubinary" "[[FATBIN]]" {{.*}}"-o" "[[HOST_O:.*o]]" {{.*}}"[[HOST_PP]]"
+// CHECK: {{".*ld.*"}} {{.*}}"[[HOST_O]]"
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu \
+// RUN:   --offload-arch=gfx803 -nogpulib -fgpu-rdc \
+// RUN:   -x hip-cpp-output %s 2>&1 | FileCheck -check-prefix=RDC %s
+
+// RDC: {{".*clang-offload-bundler.*"}} {{.*}}"-outputs=[[HOST_PP:.*cui]],[[DEV_PP:.*cui]]" "-unbundle"
+// RDC: {{".*clang.*"}} {{.*}}"-triple" "x86_64-unknown-linux-gnu"{{.*}} "-o" "[[HOST_O:.*o]]" 

[PATCH] D90173: [PowerPC] Exploit splat instruction xxsplti32dx in Power10

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

This is not functionally correct.




Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9345
+  return DAG.getBitcast(Op.getValueType(), SplatNode);
+} else { // we may lose precision, so we have to use XXSPLTI32DX.
+

Nit: full sentences in comments. Capitalize the first word.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9349
+  (uint32_t)((APSplatBits.getZExtValue() & 0xLL) >> 
32);
+  uint32_t Lo =
+  (uint32_t)(APSplatBits.getZExtValue() & 0xLL);

Won't this just produce a zero?



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9351
+  (uint32_t)(APSplatBits.getZExtValue() & 0xLL);
+  SDValue SplatNode;
+

Initialize to `undef` to simplify the logic below.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9359
+SplatNode =
+DAG.getNode(PPCISD::XXSPLTI32DX, !Hi ? SDLoc(SplatNode) : dl,
+MVT::v2i64, !Hi ? SplatNode : DAG.getUNDEF(MVT::v2i64),

The debug location comes from the original node (i.e. `dl` in this case). Just 
use that - no ternary operator.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9361
+MVT::v2i64, !Hi ? SplatNode : DAG.getUNDEF(MVT::v2i64),
+DAG.getTargetConstant(isLE ? 0 : 1, dl, MVT::i32),
+DAG.getTargetConstant(Lo, dl, MVT::i32));

Why does the index change depending on endianness? You are trying to produce a 
64-bit pattern that is in no way dependent on endianness.


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

https://reviews.llvm.org/D90173

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

[PATCH] D92977: [clangd] Improve hover and goToDefinition on auto and dectype

[Quentin Chateau via Phabricator via cfe-commits]

qchateau updated this revision to Diff 311219.
qchateau added a comment.

- Remove the patches affecting hover
- Add tests in ASTTests.cpp to test the behavior of getDeducedType for auto and 
decltype
- Add missing comment
- Create a locateSymbolForType function (which uses targetDecl)
- Remove the unit tests that were testing undesirable behaviors


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92977

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -624,6 +624,134 @@
 struct Fo^o {};
   )cpp",
 
+  R"cpp(// auto builtin type (not supported)
+^auto x = 42;
+  )cpp",
+
+  R"cpp(// auto on lambda
+auto x = [[[]]]{};
+^auto y = x;
+  )cpp",
+
+  R"cpp(// auto on struct
+namespace ns1 {
+struct [[S1]] {};
+} // namespace ns1
+
+^auto x = ns1::S1{};
+  )cpp",
+
+  R"cpp(// decltype on struct
+namespace ns1 {
+struct [[S1]] {};
+} // namespace ns1
+
+ns1::S1 i;
+^decltype(i) j;
+  )cpp",
+
+  R"cpp(// decltype(auto) on struct
+namespace ns1 {
+struct [[S1]] {};
+} // namespace ns1
+
+ns1::S1 i;
+ns1::S1& j = i;
+^decltype(auto) k = j;
+  )cpp",
+
+  R"cpp(// auto on template class
+template class [[Foo]] {};
+
+^auto x = Foo();
+  )cpp",
+
+  R"cpp(// auto on template class with forward declared class
+template class [[Foo]] {};
+class X;
+
+^auto x = Foo();
+  )cpp",
+
+  R"cpp(// auto on specialized template class
+template class Foo {};
+template<> class [[Foo]] {};
+
+^auto x = Foo();
+  )cpp",
+
+  R"cpp(// auto on initializer list.
+namespace std
+{
+  template
+  class [[initializer_list]] {};
+}
+
+^auto i = {1,2};
+  )cpp",
+
+  R"cpp(// auto function return with trailing type
+struct [[Bar]] {};
+^auto test() -> decltype(Bar()) {
+  return Bar();
+}
+  )cpp",
+
+  R"cpp(// decltype in trailing return type
+struct [[Bar]] {};
+auto test() -> ^decltype(Bar()) {
+  return Bar();
+}
+  )cpp",
+
+  R"cpp(// auto in function return
+struct [[Bar]] {};
+^auto test() {
+  return Bar();
+}
+  )cpp",
+
+  R"cpp(// auto& in function return
+struct [[Bar]] {};
+^auto& test() {
+  static Bar x;
+  return x;
+}
+  )cpp",
+
+  R"cpp(// auto* in function return
+struct [[Bar]] {};
+^auto* test() {
+  Bar* x;
+  return x;
+}
+  )cpp",
+
+  R"cpp(// const auto& in function return
+struct [[Bar]] {};
+const ^auto& test() {
+  static Bar x;
+  return x;
+}
+  )cpp",
+
+  R"cpp(// decltype(auto) in function return
+struct [[Bar]] {};
+^decltype(auto) test() {
+  return Bar();
+}
+  )cpp",
+
+  R"cpp(// decltype of function with trailing return type.
+struct [[Bar]] {};
+auto test() -> decltype(Bar()) {
+  return Bar();
+}
+void foo() {
+  ^decltype(test()) i = test();
+}
+  )cpp",
+
   R"cpp(// Override specifier jumps to overridden method
 class Y { virtual void $decl[[a]]() = 0; };
 class X : Y { void a() ^override {} };
Index: clang-tools-extra/clangd/unittests/ASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ASTTests.cpp
@@ -26,23 +26,159 @@
 namespace clangd {
 namespace {
 
-TEST(GetDeducedType, KwAutoExpansion) {
+TEST(GetDeducedType, KwAutoKwDecltypeExpansion) {
   struct Test {
 StringRef AnnotatedCode;
 const char *DeducedType;
   } Tests[] = {
   {"^auto i = 0;", "int"},
   {"^auto f(){ return 1;};", "int"},
+  {
+  R"cpp( // auto on struct in a namespace
+  namespace ns1 { struct S {}; }
+  ^auto v = ns1::S{};
+  )cpp",
+  "struct ns1::S",
+  },
+  {
+  R"cpp( // decltype on struct
+  namespace ns1 { struct S {}; }
+  ns1::S i;
+  ^decltype(i) j;
+  )cpp",
+  "ns1::S",
+  },
+  {
+  R"cpp(// decltype(auto) on struct&
+namespace ns1 {
+struct S {};
+} // namespace ns1
+
+  

[PATCH] D92940: [X86] Convert fadd/fmul _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

[Pengfei Wang via Phabricator via cfe-commits]

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

LGTM. Thanks for bringing this refactor.
I also verified that ICC and GCC both do reduce math in an binary tree way, 
though sometimes ICC has a different LSB from GCC and Clang.




Comment at: clang/lib/Headers/avx512fintrin.h:9559
 static __inline__ double __DEFAULT_FN_ATTRS512
 _mm512_reduce_max_pd(__m512d __V) {
   _mm512_mask_reduce_operator(max_pd);

Better to change min and max as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92940

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

[PATCH] D84018: Port Preprocessor and PreprocessorOutput option flags to new option parsing system

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 commandeered this revision.
jansvoboda11 added a reviewer: dang.
jansvoboda11 added a comment.

Taking over this patch, as Daniel is no longer involved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84018

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

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

ping


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

https://reviews.llvm.org/D71726

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

[PATCH] D93113: [clangd] Use enumMember instead of enumConstant

[Nathan James via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG62b4a69969c3: [clangd] Use enumMember instead of 
enumConstant (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93113

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp


Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -571,7 +571,7 @@
   case HighlightingKind::Enum:
 return "enum";
   case HighlightingKind::EnumConstant:
-return "enumConstant"; // nonstandard
+return "enumMember";
   case HighlightingKind::Typedef:
 return "type";
   case HighlightingKind::DependentType:


Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -571,7 +571,7 @@
   case HighlightingKind::Enum:
 return "enum";
   case HighlightingKind::EnumConstant:
-return "enumConstant"; // nonstandard
+return "enumMember";
   case HighlightingKind::Typedef:
 return "type";
   case HighlightingKind::DependentType:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 62b4a69 - [clangd] Use enumMember instead of enumConstant

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-12-11T14:46:13Z
New Revision: 62b4a69969c38158ce0c49803c50c32a76bbbe14

URL: 
https://github.com/llvm/llvm-project/commit/62b4a69969c38158ce0c49803c50c32a76bbbe14
DIFF: 
https://github.com/llvm/llvm-project/commit/62b4a69969c38158ce0c49803c50c32a76bbbe14.diff

LOG: [clangd] Use enumMember instead of enumConstant

We should be using enumMember as thats defined in LSP, enumConstant is non 
standard so clients aren't likely to support it
Fixes https://github.com/clangd/clangd/issues/622n

Reviewed By: sammccall

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 1022ccd0b15f..be0a5437cde6 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -571,7 +571,7 @@ llvm::StringRef toSemanticTokenType(HighlightingKind Kind) {
   case HighlightingKind::Enum:
 return "enum";
   case HighlightingKind::EnumConstant:
-return "enumConstant"; // nonstandard
+return "enumMember";
   case HighlightingKind::Typedef:
 return "type";
   case HighlightingKind::DependentType:



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

[PATCH] D92761: [clang][AArch64][SVE] Avoid going through memory for VLAT <-> VLST casts

[Joe Ellis via Phabricator via cfe-commits]

joechrisellis added reviewers: c-rhodes, aeubanks.
joechrisellis added subscribers: aeubanks, c-rhodes.
joechrisellis added a comment.

Adding reviewers @c-rhodes and @aeubanks because `git blame` tells me they've 
touched the surrounding code. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92761

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

[PATCH] D83892: [clang][cli] Port CodeGen option flags to new option parsing system

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 311217.
jansvoboda11 added a comment.

Remove whitespace change, rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83892

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -314,6 +314,7 @@
   CodeGenOpts.XRayInstrumentFunctions = LangOpts.XRayInstrument;
   CodeGenOpts.XRayAlwaysEmitCustomEvents = LangOpts.XRayAlwaysEmitCustomEvents;
   CodeGenOpts.XRayAlwaysEmitTypedEvents = LangOpts.XRayAlwaysEmitTypedEvents;
+  CodeGenOpts.DisableFree = FrontendOpts.DisableFree;
   FrontendOpts.GenerateGlobalModuleIndex = FrontendOpts.UseGlobalModuleIndex;
 
   llvm::sys::Process::UseANSIEscapeCodes(DiagOpts.UseANSIEscapeCodes);
@@ -903,22 +904,9 @@
   Opts.setDebuggerTuning(static_cast(Val));
   }
   Opts.DwarfVersion = getLastArgIntValue(Args, OPT_dwarf_version_EQ, 0, Diags);
-  Opts.DebugColumnInfo = !Args.hasArg(OPT_gno_column_info);
-  Opts.EmitCodeView = Args.hasArg(OPT_gcodeview);
-  Opts.MacroDebugInfo = Args.hasArg(OPT_debug_info_macro);
-  Opts.WholeProgramVTables = Args.hasArg(OPT_fwhole_program_vtables);
-  Opts.VirtualFunctionElimination =
-  Args.hasArg(OPT_fvirtual_function_elimination);
-  Opts.LTOVisibilityPublicStd = Args.hasArg(OPT_flto_visibility_public_std);
   Opts.SplitDwarfFile = std::string(Args.getLastArgValue(OPT_split_dwarf_file));
   Opts.SplitDwarfOutput =
   std::string(Args.getLastArgValue(OPT_split_dwarf_output));
-  Opts.SplitDwarfInlining = !Args.hasArg(OPT_fno_split_dwarf_inlining);
-  Opts.DebugTypeExtRefs = Args.hasArg(OPT_dwarf_ext_refs);
-  Opts.DebugExplicitImport = Args.hasArg(OPT_dwarf_explicit_import);
-  Opts.DebugFwdTemplateParams = Args.hasArg(OPT_debug_forward_template_params);
-  Opts.EmbedSource = Args.hasArg(OPT_gembed_source);
-  Opts.ForceDwarfFrameSection = Args.hasArg(OPT_fforce_dwarf_frame);
 
   for (const auto  : Args.getAllArgValues(OPT_fdebug_prefix_map_EQ)) {
 auto Split = StringRef(Arg).split('=');
@@ -926,13 +914,6 @@
 {std::string(Split.first), std::string(Split.second)});
   }
 
-  if (const Arg *A =
-  Args.getLastArg(OPT_emit_llvm_uselists, OPT_no_emit_llvm_uselists))
-Opts.EmitLLVMUseLists = A->getOption().getID() == OPT_emit_llvm_uselists;
-
-  Opts.DisableLLVMPasses = Args.hasArg(OPT_disable_llvm_passes);
-  Opts.DisableLifetimeMarkers = Args.hasArg(OPT_disable_lifetimemarkers);
-
   const llvm::Triple::ArchType DebugEntryValueArchs[] = {
   llvm::Triple::x86, llvm::Triple::x86_64, llvm::Triple::aarch64,
   llvm::Triple::arm, llvm::Triple::armeb, llvm::Triple::mips,
@@ -943,29 +924,12 @@
   llvm::is_contained(DebugEntryValueArchs, T.getArch()))
 Opts.EmitCallSiteInfo = true;
 
-  Opts.ValueTrackingVariableLocations =
-  Args.hasArg(OPT_fexperimental_debug_variable_locations);
-
-  Opts.DisableO0ImplyOptNone = Args.hasArg(OPT_disable_O0_optnone);
-  Opts.DisableRedZone = Args.hasArg(OPT_disable_red_zone);
-  Opts.IndirectTlsSegRefs = Args.hasArg(OPT_mno_tls_direct_seg_refs);
-  Opts.ForbidGuardVariables = Args.hasArg(OPT_fforbid_guard_variables);
-  Opts.UseRegisterSizedBitfieldAccess = Args.hasArg(
-OPT_fuse_register_sized_bitfield_access);
-  Opts.RelaxedAliasing = Args.hasArg(OPT_relaxed_aliasing);
-  Opts.StructPathTBAA = !Args.hasArg(OPT_no_struct_path_tbaa);
   Opts.NewStructPathTBAA = !Args.hasArg(OPT_no_struct_path_tbaa) &&
Args.hasArg(OPT_new_struct_path_tbaa);
-  Opts.FineGrainedBitfieldAccesses =
-  Args.hasFlag(OPT_ffine_grained_bitfield_accesses,
-   OPT_fno_fine_grained_bitfield_accesses, false);
   Opts.DwarfDebugFlags =
   std::string(Args.getLastArgValue(OPT_dwarf_debug_flags));
   Opts.RecordCommandLine =
   std::string(Args.getLastArgValue(OPT_record_command_line));
-  Opts.MergeAllConstants = Args.hasArg(OPT_fmerge_all_constants);
-  Opts.NoCommon = !Args.hasArg(OPT_fcommon);
-  Opts.NoImplicitFloat = Args.hasArg(OPT_no_implicit_float);
   Opts.OptimizeSize = getOptimizationLevelSize(Args);
   Opts.SimplifyLibCalls = !(Args.hasArg(OPT_fno_builtin) ||
 Args.hasArg(OPT_ffreestanding));
@@ -974,26 +938,17 @@
   Opts.UnrollLoops =
   Args.hasFlag(OPT_funroll_loops, OPT_fno_unroll_loops,
(Opts.OptimizationLevel > 1));
-  Opts.RerollLoops = Args.hasArg(OPT_freroll_loops);
 
-  Opts.DisableIntegratedAS = Args.hasArg(OPT_fno_integrated_as);
   Opts.SampleProfileFile =
   std::string(Args.getLastArgValue(OPT_fprofile_sample_use_EQ));
-  Opts.DebugInfoForProfiling = Args.hasFlag(
-  OPT_fdebug_info_for_profiling, OPT_fno_debug_info_for_profiling, false);
-  Opts.PseudoProbeForProfiling =
- 

[PATCH] D93094: [clang][cli] Prevent double denormalization

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 311216.
jansvoboda11 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93094

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td


Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -173,10 +173,10 @@
 
 // Marshalling info for booleans. Applied to the flag setting keypath to false.
 class MarshallingInfoBooleanFlag
+ code other_value, code other_name>
   : MarshallingInfoFlag {
   code Normalizer = "makeBooleanOptionNormalizer("#value#", "#other_value#", 
OPT_"#other_name#")";
-  code Denormalizer = "makeBooleanOptionDenormalizer("#value#", 
\""#other_spelling#"\")";
+  code Denormalizer = "makeBooleanOptionDenormalizer("#value#")";
 }
 
 // Mixins for additional marshalling attributes.
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -269,7 +269,7 @@
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fdebug-pass-manager")));
+  ASSERT_EQ(count(GeneratedArgs, "-fdebug-pass-manager"), 1);
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fno-debug-pass-manager";
 }
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -197,12 +197,11 @@
   };
 }
 
-static auto makeBooleanOptionDenormalizer(bool Value,
-  const char *OtherSpelling) {
-  return [Value, OtherSpelling](
- SmallVectorImpl , const char *Spelling,
- CompilerInvocation::StringAllocator, unsigned, bool KeyPath) {
-Args.push_back(KeyPath == Value ? Spelling : OtherSpelling);
+static auto makeBooleanOptionDenormalizer(bool Value) {
+  return [Value](SmallVectorImpl , const char *Spelling,
+ CompilerInvocation::StringAllocator, unsigned, bool KeyPath) {
+if (KeyPath == Value)
+  Args.push_back(Spelling);
   };
 }
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -338,7 +338,7 @@
   : Flag<["-"], flag.Spelling>, Flags, HelpText,
 MarshallingInfoBooleanFlag,
+   other.RecordName>,
 ImpliedByAnyOf {}
 
 // Generates TableGen records for two command line flags that control the same


Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -173,10 +173,10 @@
 
 // Marshalling info for booleans. Applied to the flag setting keypath to false.
 class MarshallingInfoBooleanFlag
+ code other_value, code other_name>
   : MarshallingInfoFlag {
   code Normalizer = "makeBooleanOptionNormalizer("#value#", "#other_value#", OPT_"#other_name#")";
-  code Denormalizer = "makeBooleanOptionDenormalizer("#value#", \""#other_spelling#"\")";
+  code Denormalizer = "makeBooleanOptionDenormalizer("#value#")";
 }
 
 // Mixins for additional marshalling attributes.
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -269,7 +269,7 @@
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fdebug-pass-manager")));
+  ASSERT_EQ(count(GeneratedArgs, "-fdebug-pass-manager"), 1);
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fno-debug-pass-manager";
 }
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -197,12 +197,11 @@
   };
 }
 
-static auto makeBooleanOptionDenormalizer(bool Value,
-  const char *OtherSpelling) {
-  return [Value, OtherSpelling](
- SmallVectorImpl , const char *Spelling,
- CompilerInvocation::StringAllocator, unsigned, bool KeyPath) {
-Args.push_back(KeyPath == Value ? Spelling : OtherSpelling);
+static auto makeBooleanOptionDenormalizer(bool Value) {
+  return 

[PATCH] D93113: [clangd] Use enumMember instead of enumConstant

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 311215.
njames93 marked an inline comment as done.
njames93 added a comment.

Remove changes to internal infrastructure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93113

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp


Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -571,7 +571,7 @@
   case HighlightingKind::Enum:
 return "enum";
   case HighlightingKind::EnumConstant:
-return "enumConstant"; // nonstandard
+return "enumMember";
   case HighlightingKind::Typedef:
 return "type";
   case HighlightingKind::DependentType:


Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -571,7 +571,7 @@
   case HighlightingKind::Enum:
 return "enum";
   case HighlightingKind::EnumConstant:
-return "enumConstant"; // nonstandard
+return "enumMember";
   case HighlightingKind::Typedef:
 return "type";
   case HighlightingKind::DependentType:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93113: [clangd] Use enumMember instead of enumConstant

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:51
   Enum,
-  EnumConstant,
+  EnumMember,
   Typedef,

sammccall wrote:
> `HighlightingKind` is an internal enum that doesn't match LSP naming, so no 
> need for changes there.
It doesn't strictly need it, But I do like the consistency of it being 
EnumMember throughout. I'll revert though as its not essential.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93113

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

[PATCH] D92775: [clang][cli] Add flexible TableGen multiclass for boolean options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 311208.
jansvoboda11 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92775

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td

Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -171,10 +171,12 @@
   code ValueExtractor = "(extractMaskValue)";
 }
 
-class MarshallingInfoBooleanFlag
+// Marshalling info for booleans. Applied to the flag setting keypath to false.
+class MarshallingInfoBooleanFlag
   : MarshallingInfoFlag {
-  code Normalizer = "makeBooleanFlagNormalizer(OPT_"#neg_name#")";
-  code Denormalizer = "makeBooleanFlagDenormalizer(\""#neg_spelling#"\")";
+  code Normalizer = "makeBooleanOptionNormalizer("#value#", "#other_value#", OPT_"#other_name#")";
+  code Denormalizer = "makeBooleanOptionDenormalizer("#value#", \""#other_spelling#"\")";
 }
 
 // Mixins for additional marshalling attributes.
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -88,7 +88,9 @@
   ASSERT_FALSE(Diags->hasErrorOccurred());
   ASSERT_TRUE(Invocation.getCodeGenOpts().Autolink);
 
-  // TODO: Test argument generation.
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fautolink";
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fno-autolink";
 }
 
 TEST_F(CommandLineTest, BoolOptionDefaultTruePresentNegChange) {
@@ -98,7 +100,9 @@
   ASSERT_FALSE(Diags->hasErrorOccurred());
   ASSERT_FALSE(Invocation.getCodeGenOpts().Autolink);
 
-  // TODO: Test argument generation.
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fno-autolink")));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fautolink";
 }
 
 TEST_F(CommandLineTest, BoolOptionDefaultTruePresentPosReset) {
@@ -120,7 +124,9 @@
   ASSERT_FALSE(Diags->hasErrorOccurred());
   ASSERT_FALSE(Invocation.getCodeGenOpts().NoInlineLineTables);
 
-  // TODO: Test argument generation.
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-ginline-line-tables";
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-gno-inline-line-tables";
 }
 
 TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNegChange) {
@@ -130,7 +136,9 @@
   ASSERT_FALSE(Diags->hasErrorOccurred());
   ASSERT_TRUE(Invocation.getCodeGenOpts().NoInlineLineTables);
 
-  // TODO: Test argument generation.
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-gno-inline-line-tables")));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-ginline-line-tables";
 }
 
 TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentPosReset) {
@@ -152,7 +160,9 @@
   ASSERT_FALSE(Diags->hasErrorOccurred());
   ASSERT_FALSE(Invocation.getCodeGenOpts().CodeViewGHash);
 
-  // TODO: Test argument generation.
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-gcodeview-ghash";
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-gno-codeview-ghash";
 }
 
 TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentPosChange) {
@@ -162,7 +172,9 @@
   ASSERT_FALSE(Diags->hasErrorOccurred());
   ASSERT_TRUE(Invocation.getCodeGenOpts().CodeViewGHash);
 
-  // TODO: Test argument generation.
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-gcodeview-ghash")));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-gno-codeview-ghash";
 }
 
 TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNegReset) {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -185,20 +185,24 @@
   return FlagToValueNormalizer{std::move(Value)};
 }
 
-static auto makeBooleanFlagNormalizer(OptSpecifier NegOpt) {
-  return [NegOpt](OptSpecifier PosOpt, unsigned, const ArgList ,
-  DiagnosticsEngine &) -> Optional {
-if (const Arg *A = Args.getLastArg(PosOpt, NegOpt))
-  return A->getOption().matches(PosOpt);
+static auto makeBooleanOptionNormalizer(bool Value, bool OtherValue,
+OptSpecifier OtherOpt) {
+  return [Value, OtherValue, OtherOpt](OptSpecifier Opt, unsigned,
+   const ArgList ,
+   

[PATCH] D92782: [CodeGen][AMDGPU] Fix ICE for static initializer IR generation

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92782

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

[PATCH] D93113: [clangd] Use enumMember instead of enumConstant

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks! Most of the changes in this patch are to the internal infrastructure, 
which predates LSP semanticTokens and doesn't need to have names that match (in 
many cases the names don't closely match C++ convention).
I think only the `toSemanticTokenType` change is needed.




Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:51
   Enum,
-  EnumConstant,
+  EnumMember,
   Typedef,

`HighlightingKind` is an internal enum that doesn't match LSP naming, so no 
need for changes there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93113

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

[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

A gentle ping :-)


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

https://reviews.llvm.org/D85984

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

[PATCH] D93113: [clangd] Use enumMember instead of enumConstant

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: sammccall, hokein.
Herald added subscribers: usaxena95, kadircet, arphaman.
njames93 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

We should be using enumMember as thats defined in LSP, enumConstant is non 
standard so clients aren't likely to support it
Fixes https://github.com/clangd/clangd/issues/622n


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93113

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -51,7 +51,7 @@
   {HighlightingKind::Class, "Class"},
   {HighlightingKind::Enum, "Enum"},
   {HighlightingKind::Namespace, "Namespace"},
-  {HighlightingKind::EnumConstant, "EnumConstant"},
+  {HighlightingKind::EnumMember, "EnumMember"},
   {HighlightingKind::Field, "Field"},
   {HighlightingKind::StaticField, "StaticField"},
   {HighlightingKind::Method, "Method"},
@@ -212,18 +212,18 @@
 )cpp",
   R"cpp(
   enum class $Enum[[E]] {
-$EnumConstant[[A]],
-$EnumConstant[[B]],
+$EnumMember[[A]],
+$EnumMember[[B]],
   };
   enum $Enum[[EE]] {
-$EnumConstant[[Hi]],
+$EnumMember[[Hi]],
   };
   struct $Class[[A]] {
 $Enum[[E]] $Field[[EEE]];
 $Enum[[EE]] $Field[[]];
   };
-  int $Variable[[I]] = $EnumConstant[[Hi]];
-  $Enum[[E]] $Variable[[L]] = $Enum[[E]]::$EnumConstant[[B]];
+  int $Variable[[I]] = $EnumMember[[Hi]];
+  $Enum[[E]] $Variable[[L]] = $Enum[[E]]::$EnumMember[[B]];
 )cpp",
   R"cpp(
   namespace $Namespace[[abc]] {
@@ -233,7 +233,7 @@
   namespace $Namespace[[cde]] {
 struct $Class[[A]] {
   enum class $Enum[[B]] {
-$EnumConstant[[Hi]],
+$EnumMember[[Hi]],
   };
 };
   }
@@ -244,7 +244,7 @@
 $Namespace[[abc]]::$Namespace[[bcd]]::$Namespace[[cde]];
   $Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[AA]];
   $Namespace[[vwz]]::$Class[[A]]::$Enum[[B]] $Variable[[AAA]] =
-$Namespace[[vwz]]::$Class[[A]]::$Enum[[B]]::$EnumConstant[[Hi]];
+$Namespace[[vwz]]::$Class[[A]]::$Enum[[B]]::$EnumMember[[Hi]];
   ::$Namespace[[vwz]]::$Class[[A]] $Variable[[B]];
   ::$Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[BB]];
 )cpp",
@@ -357,7 +357,7 @@
 )cpp",
   R"cpp(
   enum $Enum[[En]] {
-$EnumConstant[[EC]],
+$EnumMember[[EC]],
   };
   class $Class[[Foo]] {};
   class $Class[[Bar]] {
@@ -371,15 +371,15 @@
   $Field[[I]] (123) {}
   };
   class $Class[[Bar2]] : public $Class[[Bar]] {
-$Class[[Bar2]]() : $Class[[Bar]]($Class[[Foo]](), $EnumConstant[[EC]]) {}
+$Class[[Bar2]]() : $Class[[Bar]]($Class[[Foo]](), $EnumMember[[EC]]) {}
   };
 )cpp",
   R"cpp(
   enum $Enum[[E]] {
-$EnumConstant[[E]],
+$EnumMember[[E]],
   };
   class $Class[[Foo]] {};
-  $Enum[[auto]] $Variable[[AE]] = $Enum[[E]]::$EnumConstant[[E]];
+  $Enum[[auto]] $Variable[[AE]] = $Enum[[E]]::$EnumMember[[E]];
   $Class[[auto]] $Variable[[AF]] = $Class[[Foo]]();
   $Class[[decltype]](auto) $Variable[[AF2]] = $Class[[Foo]]();
   $Class[[auto]] *$Variable[[AFP]] = &$Variable[[AF]];
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -48,7 +48,7 @@
   StaticField,
   Class,
   Enum,
-  EnumConstant,
+  EnumMember,
   Typedef,
   DependentType,
   DependentName,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -82,7 +82,7 @@
   if (isa(D))
 return HighlightingKind::Enum;
   if (isa(D))
-return HighlightingKind::EnumConstant;
+return HighlightingKind::EnumMember;
   if (isa(D))
 return HighlightingKind::Parameter;
   if (auto *VD = dyn_cast(D))
@@ -427,8 +427,8 @@
 return OS << "Class";
   case HighlightingKind::Enum:
 return OS << "Enum";
-  case HighlightingKind::EnumConstant:
-return OS << "EnumConstant";
+  case HighlightingKind::EnumMember:
+return OS << "EnumMember";
   case HighlightingKind::Typedef:
 return OS << "Typedef";
   

[PATCH] D93110: [analyzer] Implement a first version of suppressions via attributes

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Right now, I reused existing `suppress` attribute.  However I did it in a 
rather unconventional manner.  I allowed 0 arguments in one spelling and >1 in 
another, which seems odd.
I can see a couple other possible solutions here:

- Choose a "keyword" that would be used to suppress all warnings (e.g. 
"suppress(all)")
- Introduce two different suppress attributes (maybe even rename existing 
attribute to be GSLSuppress)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93110

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

[PATCH] D92857: [clang][cli] [clang][cli] Don't always emit -f[no-]legacy-pass-manager

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 311201.
jansvoboda11 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92857

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td


Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -173,7 +173,6 @@
 
 class MarshallingInfoBooleanFlag
   : MarshallingInfoFlag {
-  bit ShouldAlwaysEmit = 1;
   code Normalizer = "makeBooleanFlagNormalizer(OPT_"#neg_name#")";
   code Denormalizer = "makeBooleanFlagDenormalizer(\""#neg_spelling#"\")";
 }
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -198,7 +198,7 @@
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq(PassManagerResetByFlag)));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerResetByFlag;
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerChangedByFlag;
 }
 
@@ -222,7 +222,7 @@
   ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager, PassManagerDefault);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq(PassManagerResetByFlag)));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerResetByFlag;
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerChangedByFlag;
 }
 


Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -173,7 +173,6 @@
 
 class MarshallingInfoBooleanFlag
   : MarshallingInfoFlag {
-  bit ShouldAlwaysEmit = 1;
   code Normalizer = "makeBooleanFlagNormalizer(OPT_"#neg_name#")";
   code Denormalizer = "makeBooleanFlagDenormalizer(\""#neg_spelling#"\")";
 }
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -198,7 +198,7 @@
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq(PassManagerResetByFlag)));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerResetByFlag;
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerChangedByFlag;
 }
 
@@ -222,7 +222,7 @@
   ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager, PassManagerDefault);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq(PassManagerResetByFlag)));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerResetByFlag;
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerChangedByFlag;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92774: [clang][cli] CompilerInvocationTest: add tests for boolean options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 311198.
jansvoboda11 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92774

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp

Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -77,6 +77,155 @@
   ASSERT_TRUE(Invocation.getFrontendOpts().UseTemporary);
 }
 
+// Boolean option with a keypath that defaults to true.
+// The flag with negative spelling can set the keypath to false.
+// The flag with positive spelling can reset the keypath to true.
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentNone) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().Autolink);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentNegChange) {
+  const char *Args[] = {"-fno-autolink"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().Autolink);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentPosReset) {
+  const char *Args[] = {"-fautolink"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_TRUE(Invocation.getCodeGenOpts().Autolink);
+}
+
+// Boolean option with a keypath that defaults to false.
+// The flag with negative spelling can set the keypath to true.
+// The flag with positive spelling can reset the keypath to false.
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNone) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().NoInlineLineTables);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNegChange) {
+  const char *Args[] = {"-gno-inline-line-tables"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().NoInlineLineTables);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentPosReset) {
+  const char *Args[] = {"-ginline-line-tables"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_FALSE(Invocation.getCodeGenOpts().NoInlineLineTables);
+}
+
+// Boolean option with a keypath that defaults to false.
+// The flag with positive spelling can set the keypath to true.
+// The flag with negative spelling can reset the keypath to false.
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNoneX) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().CodeViewGHash);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentPosChange) {
+  const char *Args[] = {"-gcodeview-ghash"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().CodeViewGHash);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNegReset) {
+  const char *Args[] = {"-gno-codeview-ghash"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_FALSE(Invocation.getCodeGenOpts().CodeViewGHash);
+}
+
+// Boolean option with a keypath that defaults to an arbitrary expression.
+// The flag with positive spelling can set the keypath to true.
+// The flag with negative spelling can set the keypath to false.
+
+static constexpr unsigned PassManagerDefault =
+!static_cast(LLVM_ENABLE_NEW_PASS_MANAGER);
+
+static constexpr const char *PassManagerResetByFlag =
+LLVM_ENABLE_NEW_PASS_MANAGER ? "-fno-legacy-pass-manager"
+ : "-flegacy-pass-manager";
+
+static constexpr const char *PassManagerChangedByFlag =
+LLVM_ENABLE_NEW_PASS_MANAGER ? "-flegacy-pass-manager"
+ : "-fno-legacy-pass-manager";
+
+TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentNone) {
+  const char *Args = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager, PassManagerDefault);
+
+  

[PATCH] D93110: [analyzer] Implement a first version of suppressions via attributes

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko created this revision.
Herald added subscribers: steakhal, ASDenysPetrov, martong, Charusso, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, 
xazax.hun.
Herald added a reviewer: aaron.ballman.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

At the moment, the only possible way to suppress analyzer warnings
is by cutting the code out with #ifdef.  It is practically impossible
to do locally, so whole functions should stay not analyzed.

This patch suggests how we can use `__attribute__((suppress))`
for these purposes.  Even without specifying checker IDs, it is
far more precise than the existing practice.  It can be applied
to statements in question.  Its proximity to the warning and
the fact that this is part of the code, drastically increases
its chances to survive code refactoring.

Further on, when we decide on stable checker identifiers, we
can implement finer approach, where the user can specify those
to suppress specific checks.  Current code still can be relevant
as it gives "suppress all" option, which is usally enough.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93110

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/test/Analysis/suppression-attr.m
  clang/test/SemaCXX/suppress.cpp

Index: clang/test/SemaCXX/suppress.cpp
===
--- clang/test/SemaCXX/suppress.cpp
+++ clang/test/SemaCXX/suppress.cpp
@@ -6,17 +6,20 @@
   [[gsl::suppress("in-a-namespace")]];
 }
 
-[[gsl::suppress("readability-identifier-naming")]]
-void f_() {
+[[gsl::suppress("readability-identifier-naming")]] void f_() {
   int *p;
   [[gsl::suppress("type", "bounds")]] {
 p = reinterpret_cast(7);
   }
 
-  [[gsl::suppress]] int x; // expected-error {{'suppress' attribute takes at least 1 argument}}
-  [[gsl::suppress()]] int y; // expected-error {{'suppress' attribute takes at least 1 argument}}
-  int [[gsl::suppress("r")]] z; // expected-error {{'suppress' attribute cannot be applied to types}}
+  [[gsl::suppress]] int x;   // expected-error {{'suppress' attribute takes at least 1 argument}}
+  [[gsl::suppress()]] int y; // expected-error {{'suppress' attribute takes at least 1 argument}}
+  int [[gsl::suppress("r")]] z;  // expected-error {{'suppress' attribute cannot be applied to types}}
   [[gsl::suppress(f_)]] float f; // expected-error {{'suppress' attribute requires a string}}
+  // Only gsl spelling requires at least 1 argument
+  __attribute__((suppress)) double d;   // no-error
+  __attribute__((suppress())) double e; // no-error
+  __attribute__((suppress)) { int a; }  // no-error
 }
 
 union [[gsl::suppress("type.1")]] U {
Index: clang/test/Analysis/suppression-attr.m
===
--- /dev/null
+++ clang/test/Analysis/suppression-attr.m
@@ -0,0 +1,110 @@
+// RUN: %clang_analyze_cc1 -fblocks \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=osx.cocoa.MissingSuperCall \
+// RUN:   -analyzer-checker=osx.cocoa.NSError \
+// RUN:   -Wno-unused-value -Wno-objc-root-class -verify %s
+
+#define SUPPRESS __attribute__((suppress))
+
+@protocol NSObject
+- (id)retain;
+- (oneway void)release;
+@end
+@interface NSObject  {
+}
+- (id)init;
++ (id)alloc;
+@end
+typedef int NSInteger;
+typedef char BOOL;
+typedef struct _NSZone NSZone;
+@class NSInvocation, NSMethodSignature, NSCoder, NSString, NSEnumerator;
+@protocol NSCopying
+- (id)copyWithZone:(NSZone *)zone;
+@end
+@protocol NSCoding
+- (void)encodeWithCoder:(NSCoder *)aCoder;
+@end
+@class NSDictionary;
+@interface NSError : NSObject  {
+}
++ (id)errorWithDomain:(NSString *)domain code:(NSInteger)code userInfo:(NSDictionary *)dict;
+@end
+
+void dereference_1() {
+  int *x = 0;
+  *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+}
+
+void dereference_suppression_1() {
+  int *x = 0;
+  SUPPRESS { *x; } // no-warning
+}
+
+void dereference_2() {
+  int *x = 0;
+  if (*x) { // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+  }
+}
+
+void dereference_suppression_2() {
+  int *x = 0;
+  SUPPRESS if (*x) { // no-warning
+  }
+}
+
+void dereference_3(int cond) {
+  int *x = 0;
+  if (cond) {
+(*x)++; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+  }
+}
+
+void dereference_suppression_3(int cond) {
+  int *x = 0;
+  SUPPRESS if (cond) {
+(*x)++; // no-warning
+  }
+}
+
+void dereference_4() {
+  int *x = 0;
+  int y = *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+}
+
+void dereference_suppression_4() {
+  int *x = 0;
+  SUPPRESS int y = *x; // no-warning
+}
+
+@interface UIResponder : NSObject {
+}
+- (char)resignFirstResponder;
+@end
+

[PATCH] D92857: [clang][cli] [clang][cli] Don't always emit -f[no-]legacy-pass-manager

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 311193.
jansvoboda11 added a comment.

Extract default value


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92857

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td


Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -173,7 +173,6 @@
 
 class MarshallingInfoBooleanFlag
   : MarshallingInfoFlag {
-  bit ShouldAlwaysEmit = 1;
   code Normalizer = "makeBooleanFlagNormalizer(OPT_"#neg_name#")";
   code Denormalizer = "makeBooleanFlagDenormalizer(\""#neg_spelling#"\")";
 }
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -177,51 +177,53 @@
 // The flag with positive spelling can set the keypath to true.
 // The flag with negative spelling can set the keypath to false.
 
+static constexpr unsigned PassManagerDefault =
+!static_cast(LLVM_ENABLE_NEW_PASS_MANAGER);
+
+static constexpr const char *PassManagerResetByFlag =
+LLVM_ENABLE_NEW_PASS_MANAGER ? "-fno-legacy-pass-manager"
+ : "-flegacy-pass-manager";
+
+static constexpr const char *PassManagerChangedByFlag =
+LLVM_ENABLE_NEW_PASS_MANAGER ? "-flegacy-pass-manager"
+ : "-fno-legacy-pass-manager";
+
 TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentNone) {
   const char *Args = {""};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
   ASSERT_FALSE(Diags->hasErrorOccurred());
-  ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager,
-!static_cast(LLVM_ENABLE_NEW_PASS_MANAGER));
+  ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager, PassManagerDefault);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
-  const char *ResetByFlag = LLVM_ENABLE_NEW_PASS_MANAGER
-? "-fno-legacy-pass-manager"
-: "-flegacy-pass-manager";
-
-  const char *ChangedByFlag = LLVM_ENABLE_NEW_PASS_MANAGER
-  ? "-flegacy-pass-manager"
-  : "-fno-legacy-pass-manager";
-
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq(ResetByFlag)));
-  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(ChangedByFlag;
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerResetByFlag;
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerChangedByFlag;
 }
 
-TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentPos) {
-  const char *Args[] = {"-flegacy-pass-manager"};
+TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentChange) {
+  const char *Args[] = {PassManagerChangedByFlag};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
   ASSERT_FALSE(Diags->hasErrorOccurred());
-  ASSERT_TRUE(Invocation.getCodeGenOpts().LegacyPassManager);
+  ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager, 
!PassManagerDefault);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-flegacy-pass-manager")));
-  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fno-legacy-pass-manager";
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq(PassManagerChangedByFlag)));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerResetByFlag;
 }
 
-TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentNeg) {
-  const char *Args[] = {"-fno-legacy-pass-manager"};
+TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentReset) {
+  const char *Args[] = {PassManagerResetByFlag};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
   ASSERT_FALSE(Diags->hasErrorOccurred());
-  ASSERT_FALSE(Invocation.getCodeGenOpts().LegacyPassManager);
+  ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager, PassManagerDefault);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fno-legacy-pass-manager")));
-  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-flegacy-pass-manager";
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerChangedByFlag;
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerResetByFlag;
 }
 
 TEST_F(CommandLineTest, CanGenerateCC1CommandLineFlag) {


Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -173,7 +173,6 @@
 
 class MarshallingInfoBooleanFlag
   : MarshallingInfoFlag {
-  bit ShouldAlwaysEmit = 1;
   code Normalizer = "makeBooleanFlagNormalizer(OPT_"#neg_name#")";
   

[PATCH] D92857: [clang][cli] Don't always emit -f[no-]experimental-new-pass-manager

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 311190.
jansvoboda11 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92857

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td


Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -173,7 +173,6 @@
 
 class MarshallingInfoBooleanFlag
   : MarshallingInfoFlag {
-  bit ShouldAlwaysEmit = 1;
   code Normalizer = "makeBooleanFlagNormalizer(OPT_"#neg_name#")";
   code Denormalizer = "makeBooleanFlagDenormalizer(\""#neg_spelling#"\")";
 }
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -177,6 +177,14 @@
 // The flag with positive spelling can set the keypath to true.
 // The flag with negative spelling can set the keypath to false.
 
+static constexpr const char *PassManagerResetByFlag =
+LLVM_ENABLE_NEW_PASS_MANAGER ? "-fno-legacy-pass-manager"
+ : "-flegacy-pass-manager";
+
+static constexpr const char *PassManagerChangedByFlag =
+LLVM_ENABLE_NEW_PASS_MANAGER ? "-flegacy-pass-manager"
+ : "-fno-legacy-pass-manager";
+
 TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentNone) {
   const char *Args = {""};
 
@@ -188,40 +196,32 @@
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
-  const char *ResetByFlag = LLVM_ENABLE_NEW_PASS_MANAGER
-? "-fno-legacy-pass-manager"
-: "-flegacy-pass-manager";
-
-  const char *ChangedByFlag = LLVM_ENABLE_NEW_PASS_MANAGER
-  ? "-flegacy-pass-manager"
-  : "-fno-legacy-pass-manager";
-
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq(ResetByFlag)));
-  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(ChangedByFlag;
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerResetByFlag;
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerChangedByFlag;
 }
 
-TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentPos) {
-  const char *Args[] = {"-flegacy-pass-manager"};
+TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentChange) {
+  const char *Args[] = {PassManagerChangedByFlag};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
   ASSERT_FALSE(Diags->hasErrorOccurred());
   ASSERT_TRUE(Invocation.getCodeGenOpts().LegacyPassManager);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-flegacy-pass-manager")));
-  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fno-legacy-pass-manager";
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq(PassManagerChangedByFlag)));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerResetByFlag;
 }
 
-TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentNeg) {
-  const char *Args[] = {"-fno-legacy-pass-manager"};
+TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentReset) {
+  const char *Args[] = {PassManagerResetByFlag};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
   ASSERT_FALSE(Diags->hasErrorOccurred());
   ASSERT_FALSE(Invocation.getCodeGenOpts().LegacyPassManager);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
-  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fno-legacy-pass-manager")));
-  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-flegacy-pass-manager";
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerChangedByFlag;
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq(PassManagerResetByFlag;
 }
 
 TEST_F(CommandLineTest, CanGenerateCC1CommandLineFlag) {


Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -173,7 +173,6 @@
 
 class MarshallingInfoBooleanFlag
   : MarshallingInfoFlag {
-  bit ShouldAlwaysEmit = 1;
   code Normalizer = "makeBooleanFlagNormalizer(OPT_"#neg_name#")";
   code Denormalizer = "makeBooleanFlagDenormalizer(\""#neg_spelling#"\")";
 }
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -177,6 +177,14 @@
 // The flag with positive spelling can set the keypath to true.
 // The flag with negative spelling can set the keypath to false.
 
+static constexpr const char *PassManagerResetByFlag =
+LLVM_ENABLE_NEW_PASS_MANAGER ? 

[PATCH] D92954: [clang-offload-bundler] Add option -list

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 311187.
yaxunl marked 3 inline comments as done.
yaxunl added a comment.

Revised by Artem's comments.


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

https://reviews.llvm.org/D92954

Files:
  clang/test/Driver/clang-offload-bundler.c
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -62,11 +62,11 @@
cl::desc("[,...]"),
cl::cat(ClangOffloadBundlerCategory));
 static cl::list
-OutputFileNames("outputs", cl::CommaSeparated, cl::OneOrMore,
+OutputFileNames("outputs", cl::CommaSeparated,
 cl::desc("[,...]"),
 cl::cat(ClangOffloadBundlerCategory));
 static cl::list
-TargetNames("targets", cl::CommaSeparated, cl::OneOrMore,
+TargetNames("targets", cl::CommaSeparated,
 cl::desc("[-,...]"),
 cl::cat(ClangOffloadBundlerCategory));
 static cl::opt
@@ -89,6 +89,10 @@
  cl::desc("Unbundle bundled file into several output files.\n"),
  cl::init(false), cl::cat(ClangOffloadBundlerCategory));
 
+static cl::opt
+ListBundleIDs("list", cl::desc("List bundle IDs in the bundled file.\n"),
+  cl::init(false), cl::cat(ClangOffloadBundlerCategory));
+
 static cl::opt PrintExternalCommands(
 "###",
 cl::desc("Print any external commands that are to be executed "
@@ -127,6 +131,10 @@
 /// Generic file handler interface.
 class FileHandler {
 public:
+  struct BundleInfo {
+StringRef BundleID;
+  };
+
   FileHandler() {}
 
   virtual ~FileHandler() {}
@@ -163,6 +171,48 @@
 
   /// Write the bundle from \a Input into \a OS.
   virtual Error WriteBundle(raw_fd_ostream , MemoryBuffer ) = 0;
+
+  /// List bundle IDs in \a Input.
+  virtual Error listBundleIDs(MemoryBuffer ) {
+if (Error Err = ReadHeader(Input))
+  return Err;
+
+return forEachBundle(Input, [&](const BundleInfo ) -> Error {
+  llvm::outs() << Info.BundleID << '\n';
+  Error Err = listBundleIDsCallback(Input, Info);
+  if (Err)
+return Err;
+  return Error::success();
+});
+  }
+
+  /// For each bundle in \a Input, do \a Func.
+  Error forEachBundle(MemoryBuffer ,
+  std::function Func) {
+while (true) {
+  Expected> CurTripleOrErr = ReadBundleStart(Input);
+  if (!CurTripleOrErr)
+return CurTripleOrErr.takeError();
+
+  // No more bundles.
+  if (!*CurTripleOrErr)
+break;
+
+  StringRef CurTriple = **CurTripleOrErr;
+  assert(!CurTriple.empty());
+
+  BundleInfo Info{CurTriple};
+  if (Error Err = Func(Info))
+return Err;
+}
+return Error::success();
+  }
+
+protected:
+  virtual Error listBundleIDsCallback(MemoryBuffer ,
+  const BundleInfo ) {
+return Error::success();
+  }
 };
 
 /// Handler for binary files. The bundled file will have the following format
@@ -212,22 +262,23 @@
 
 class BinaryFileHandler final : public FileHandler {
   /// Information about the bundles extracted from the header.
-  struct BundleInfo final {
+  struct BinaryBundleInfo final : public BundleInfo {
 /// Size of the bundle.
 uint64_t Size = 0u;
 /// Offset at which the bundle starts in the bundled file.
 uint64_t Offset = 0u;
 
-BundleInfo() {}
-BundleInfo(uint64_t Size, uint64_t Offset) : Size(Size), Offset(Offset) {}
+BinaryBundleInfo() {}
+BinaryBundleInfo(uint64_t Size, uint64_t Offset)
+: Size(Size), Offset(Offset) {}
   };
 
   /// Map between a triple and the corresponding bundle information.
-  StringMap BundlesInfo;
+  StringMap BundlesInfo;
 
   /// Iterator for the bundle information that is being read.
-  StringMap::iterator CurBundleInfo;
-  StringMap::iterator NextBundleInfo;
+  StringMap::iterator CurBundleInfo;
+  StringMap::iterator NextBundleInfo;
 
   /// Current bundle target to be written.
   std::string CurWriteBundleTarget;
@@ -297,7 +348,7 @@
 
   assert(BundlesInfo.find(Triple) == BundlesInfo.end() &&
  "Triple is duplicated??");
-  BundlesInfo[Triple] = BundleInfo(Size, Offset);
+  BundlesInfo[Triple] = BinaryBundleInfo(Size, Offset);
 }
 // Set the iterator to where we will start to read.
 CurBundleInfo = BundlesInfo.end();
@@ -351,7 +402,7 @@
   Write8byteIntegerToBuffer(OS, HeaderSize);
   // Size of the bundle (adds to the next bundle's offset)
   Write8byteIntegerToBuffer(OS, MB.getBufferSize());
-  BundlesInfo[T] = BundleInfo(MB.getBufferSize(), HeaderSize);
+  BundlesInfo[T] = BinaryBundleInfo(MB.getBufferSize(), HeaderSize);
   HeaderSize += MB.getBufferSize();
   // Size of the triple

[PATCH] D92774: [clang][cli] CompilerInvocationTest: add tests for boolean options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 requested review of this revision.
jansvoboda11 added a comment.

@dexonsmith could you look at this again? I've now reworked the pass manager 
tests. They now work regardless of the `LLVM_ENABLE_NEW_PASS_MANAGER` value and 
use the new `-f[no-]legacy-pass-manager` flags.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92774

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

[PATCH] D92954: [clang-offload-bundler] Add option -list

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:188
+
+  if (Error Err = Func())
+return Err;

tra wrote:
> Now, if we could save the triple in a `BundleInfo` when it's parsed, and pass 
> `BundleInfo` to `Func()` that would make the iterator more useful.
> 
done



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:320-321
   ReadChars += TripleSize;
+  if (ListBundleIDs)
+llvm::outs() << Triple << '\n';
 

tra wrote:
> I'm still not quite happy with the fact that the listing is interleaving with 
> reading the bundle.
> I think the code would benefit from further refactoring that would separate 
> bundle reading from what we do with the bundles we've read.
> 
good point. this can be moved to the lambda without incurring significant 
overhead.



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:641
+// start of each bundle which is done by forEachBundle.
+return forEachBundle(Input, []() { return Error::success(); });
+  }

tra wrote:
> Once BundleInfo carries the triple, `ListBundleIDs` could then be changed to 
> print the bundle info and moved into the FileHandler class:
> 
> ```
> if (Error Err = ReadHeader(Input))
> return Err;
> 
> return forEachBundle(Input, [](BundleInfo bundle) { llvm::outs() << 
> bundle.triple << '\n'});
> ```
> 
> All other printouts of the triple should no longer be necessary.
done


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

https://reviews.llvm.org/D92954

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

[PATCH] D43002: [CodeView] Emit S_OBJNAME record

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I think the biggest concern here is what to do about `/Fa` / `-save-temps`. If 
we do nothing, the S_OBJNAME record will change if you run the same compilation 
twice with and without those flags. Generally, we strive to ensure that the 
directly emitted object file is identical to one rinsed through textual 
assembly, but that doesn't always work. I'm not sure if `/Fa` works reliably 
since we switched to Intel style assembly syntax either.




Comment at: clang/include/clang/Basic/CodeGenOptions.h:212
+  /// Output filename used in the COFF debug information.
+  std::string COFFOutputFilename;
+

Let's bikeshed the name a bit. This thing is the `-o`/`/Fo` value, plus some 
processing. It can be symbolic, as in `-`. It could be the name of a bitcode 
file with `-flto`. It could be the name of an assembly file if you do `clang -S 
--target=x86_64-windows-msvc -g t.cpp -o t.s`. It could be the name of an ELF 
file if you try hard. I think in any of these cases where the user directly 
emits something that isn't a COFF object, it's OK to use that name for the 
S_OBJNAME record.

What it is, is the name of the compiler's output file, as we would like it to 
appear in debug info. With that in mind, here are some ideas:
- CodeViewObjectName: very directly referring to S_OBJNAME
- ObjectFilename: very general
- ObjectFilenameForDebug: generalizing over cv/dwarf
- OutputFilenameForDebug: generalizing over assembly, bc, and obj

I think I like ObjectFilenameForDebug best. DebugObjectFilename seems like a 
no-go, since that sounds like the dwo file name.

---

This brings me to the case of `-save-temps` / `/Fa`. In these cases, the 
compile action is broken into pieces, and the assembler is invoked in a 
follow-up process. Does that mean the driver needs to pass an extra flag along 
to the -cc1 action? That would be a bummer.




Comment at: clang/lib/Frontend/CompilerInstance.cpp:717
+
+  getCodeGenOpts().COFFOutputFilename = OutputPathName;
+

I think saving the output file name during output file creation seems like an 
unexpected side effect for this function. Can we handle it earlier, maybe near 
dwo file name handling?



Comment at: clang/test/CodeGenCXX/debug-info-objname.cpp:4
+
+// No output file provided, input file is relative, we emit an absolute path 
(MSVC behavior).
+// RUN: %clang_cl /c /Z7 -nostdinc debug-info-objname.cpp

I checked, this is also consistent with clang's regular /Z7 output for source 
filenames.



Comment at: clang/test/CodeGenCXX/debug-info-objname.cpp:20-21
+
+// The input file name is relative and we specify -fdebug-compilation-dir, we 
emit a relative path.
+// RUN: %clang_cl /c /Z7 -nostdinc -fdebug-compilation-dir=. 
debug-info-objname.cpp
+// RUN: llvm-pdbutil dump -all debug-info-objname.obj | FileCheck %s 
--check-prefix=RELATIVE

And this is the thing we rely on, so it should all work.



Comment at: llvm/include/llvm/MC/MCTargetOptions.h:63
   std::string SplitDwarfFile;
+  std::string COFFOutputFilename;
 

This should probably be in llvm/include/llvm/Target/TargetOptions.h. I think 
MCTargetOptions is intended to control assembler behavior, but this option is 
read while producing assembly in CodeGen, not during assembling.



Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:752
 
+static void unescapeSlashes(SmallVectorImpl ) {
+  auto Read = Str.begin();

This isn't unescaping them, it's just canonicalizing double slashes to one 
slash, right? Would `llvm::sys::native` suffice?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D43002

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

[PATCH] D92774: [clang][cli] CompilerInvocationTest: add tests for boolean options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 311183.
jansvoboda11 added a comment.

Remove comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92774

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp

Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -77,6 +77,153 @@
   ASSERT_TRUE(Invocation.getFrontendOpts().UseTemporary);
 }
 
+// Boolean option with a keypath that defaults to true.
+// The flag with negative spelling can set the keypath to false.
+// The flag with positive spelling can reset the keypath to true.
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentNone) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().Autolink);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentNegChange) {
+  const char *Args[] = {"-fno-autolink"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().Autolink);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentPosReset) {
+  const char *Args[] = {"-fautolink"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_TRUE(Invocation.getCodeGenOpts().Autolink);
+}
+
+// Boolean option with a keypath that defaults to false.
+// The flag with negative spelling can set the keypath to true.
+// The flag with positive spelling can reset the keypath to false.
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNone) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().NoInlineLineTables);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNegChange) {
+  const char *Args[] = {"-gno-inline-line-tables"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().NoInlineLineTables);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentPosReset) {
+  const char *Args[] = {"-ginline-line-tables"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_FALSE(Invocation.getCodeGenOpts().NoInlineLineTables);
+}
+
+// Boolean option with a keypath that defaults to false.
+// The flag with positive spelling can set the keypath to true.
+// The flag with negative spelling can reset the keypath to false.
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNoneX) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().CodeViewGHash);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentPosChange) {
+  const char *Args[] = {"-gcodeview-ghash"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().CodeViewGHash);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNegReset) {
+  const char *Args[] = {"-gno-codeview-ghash"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_FALSE(Invocation.getCodeGenOpts().CodeViewGHash);
+}
+
+// Boolean option with a keypath that defaults to an arbitrary expression.
+// The flag with positive spelling can set the keypath to true.
+// The flag with negative spelling can set the keypath to false.
+
+TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentNone) {
+  const char *Args = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager,
+!static_cast(LLVM_ENABLE_NEW_PASS_MANAGER));
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+
+  const char *ResetByFlag = LLVM_ENABLE_NEW_PASS_MANAGER
+? "-fno-legacy-pass-manager"
+: "-flegacy-pass-manager";
+
+  const char *ChangedByFlag = LLVM_ENABLE_NEW_PASS_MANAGER
+  ? "-flegacy-pass-manager"
+  : 

[PATCH] D92774: [clang][cli] CompilerInvocationTest: add tests for boolean options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 311182.
jansvoboda11 added a comment.
This revision is now accepted and ready to land.

Rebase, fix tests and update them to use the new `-f[no-]legacy-pass-manager`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92774

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp

Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -77,6 +77,156 @@
   ASSERT_TRUE(Invocation.getFrontendOpts().UseTemporary);
 }
 
+// Boolean option with a keypath that defaults to true.
+// The flag with negative spelling can set the keypath to false.
+// The flag with positive spelling can reset the keypath to true.
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentNone) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().Autolink);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentNegChange) {
+  const char *Args[] = {"-fno-autolink"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().Autolink);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultTruePresentPosReset) {
+  const char *Args[] = {"-fautolink"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_TRUE(Invocation.getCodeGenOpts().Autolink);
+}
+
+// Boolean option with a keypath that defaults to false.
+// The flag with negative spelling can set the keypath to true.
+// The flag with positive spelling can reset the keypath to false.
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNone) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().NoInlineLineTables);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNegChange) {
+  const char *Args[] = {"-gno-inline-line-tables"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().NoInlineLineTables);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentPosReset) {
+  const char *Args[] = {"-ginline-line-tables"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_FALSE(Invocation.getCodeGenOpts().NoInlineLineTables);
+}
+
+// Boolean option with a keypath that defaults to false.
+// The flag with positive spelling can set the keypath to true.
+// The flag with negative spelling can reset the keypath to false.
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNoneX) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_FALSE(Invocation.getCodeGenOpts().CodeViewGHash);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentPosChange) {
+  const char *Args[] = {"-gcodeview-ghash"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getCodeGenOpts().CodeViewGHash);
+
+  // TODO: Test argument generation.
+}
+
+TEST_F(CommandLineTest, BoolOptionDefaultFalsePresentNegReset) {
+  const char *Args[] = {"-gno-codeview-ghash"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+  ASSERT_TRUE(Diags->hasErrorOccurred()); // Driver-only flag.
+  ASSERT_FALSE(Invocation.getCodeGenOpts().CodeViewGHash);
+}
+
+// Boolean option with a keypath that defaults to an arbitrary expression.
+// The flag with positive spelling can set the keypath to true.
+// The flag with negative spelling can set the keypath to false.
+
+// NOTE: The following tests need to be updated when we start enabling the new
+// pass manager by default.
+
+TEST_F(CommandLineTest, BoolOptionDefaultArbitraryTwoFlagsPresentNone) {
+  const char *Args = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getCodeGenOpts().LegacyPassManager,
+!static_cast(LLVM_ENABLE_NEW_PASS_MANAGER));
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+
+  const char *ResetByFlag = LLVM_ENABLE_NEW_PASS_MANAGER
+? "-fno-legacy-pass-manager"
+  

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D89909#2442573 , @bader wrote:

> In D89909#2439837 , @Anastasia wrote:
>
>> In D89909#2423750 , @bader wrote:
>>
 It was mentioned that the changes in the type system with address spaces 
 is undesirable for SYCL because you indend to parse existing C++ code 
 as-is. This contradicts the intended semantic of address spaces where the 
 whole point of it is to modify the standard types and therefore a 
 compilation of C++ with the standard semantic is only guaranteed when the 
 attribute is not used at all.
>>>
>>> Right, but I don't think it's related to the address space attributes. It 
>>> was mentioned in the context of re-using OpenCL *mode* for SYCL device 
>>> compilation, which modifies types which does use address space attribute 
>>> explicitly. "Existing C++ code" doesn't use address space attributes and 
>>> our solution does differentiate explicitly annotated type. The difference 
>>> with OpenCL mode is that SYCL doesn't change types by modifying "default" 
>>> address space attribute and allows conversion from/to "default" address 
>>> space. As I mentioned in RFC, according to my understanding this patch 
>>> doesn't contradict Embedded C requirements regarding address space 
>>> attribute usage. I think the spec allows an implementation to define 
>>> conversion rules between address spaces and doesn't imply type change based 
>>> on the declaration scope - it's OpenCL specific behavior.
>>
>> Ok, if you plan to follow the Embedded C semantic (which your current patch 
>> confirms) then you should just reuse the existing target address spaces and 
>> extend the implementation with the ability to specify the relation among 
>> address spaces. The patch that you have mentioned earlier 
>> https://reviews.llvm.org/D62574 is providing this logic already and it looks 
>> good aside from testing. I would suggest to discuss with @ebevhan the 
>> timeline for committing it as the testing could be done using SYCL 
>> implementation. Alternatively, you could consider reusing the relevant parts 
>> of the patch if @ebevhan has no objections to that.
>
> Let me check that I understand your proposal correctly.
> Do suggest that we use `__attribute__((address_space(N)))` attribute and 
> define the relation between the set of these? Unfortunately I won't work 
> because SYCL supports multiple targets and we need to use address space maps 
> to correctly map attributes in LLVM IR. Targets might use different different 
> llvm address spaces numbers for memory accessible within a work-item. If I 
> understand it correctly, `__attribute__((address_space(N)))` will be mapped 
> to `addrspace(N)` in LLVM and this mapping can't be customized for different 
> targets. Right?

Yes, this is true. However your libraries can have vendor dependent definitions 
of the target address spaces i.e. the exact number can be set per vendor. Then 
you would still need to specify the relationship of address spaces for each 
vendor in clang. I don't know whether this matches well though. It feels more 
to me that your concept is closer to address spaces defined by language 
dialects like OpenCL. If you prefer to continue this route then you should just 
document your dialect sematic somewhere publicly accessible and avoid aliasing 
to OpenCL, or target address spaces. So it would become a proper language 
dialect implementation.

> I actually proposed a bit different approach in RFC. 
> https://reviews.llvm.org/D62574 "moves QualType/Qualifiers accessors which 
> deal with qualifier relations (such as compatiblyIncludes, etc.) to 
> ASTContext, as Qualifiers cannot be made aware of the relations between 
> address spaces on their own". It allows customization based on the language 
> mode as well, so my suggestion was to set "default" address space as a 
> superset for opencl_global, opencl_local and opencl_private in SYCL mode. 
> Using this approach we re-use existing attributes and don't impact other 
> language modes. Does it make sense to you?

The default address spaces in clang is used for flat addressing of C and C++.  
Although in early OpenCL versions we have used it for `opencl_private`, that 
mainly aligned with the concept of Embedded C until in OpenCL 2.0 the semantic 
of address spaces has been modified significantly. So currently I would say 
none of the OpenCL address space semantic matches the default address space of 
clang.

Btw in the original RFC of  https://reviews.llvm.org/D62574, we have also 
discussed to extend the concept of target address spaces on language dialects 
that would follow embedded C semantic but the address spaces would work on a 
variety of targets. We were even thinking of defining a language syntax that 
would allow expressing the relationship between the address spaces in the 
source code parsed by clang