[clang] ce16c3c - [Clang][Docs] Consolidate option hiding in ClangOptionDocEmitter

[Justin Bogner via cfe-commits]

Author: Justin Bogner
Date: 2023-08-13T23:58:27-07:00
New Revision: ce16c3cf30f5e63a395927ede95161393383d636

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

LOG: [Clang][Docs] Consolidate option hiding in ClangOptionDocEmitter

Update the `hasFlag` check to account for an Option's groups to better
match how the option parsing logic works, and instead of checking if a
group has include/exclude flags just check if there are any visible
options in it.

This cleans up some the empty sections that are currently emitted in
clang's option docs.

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

Added: 


Modified: 
clang/utils/TableGen/ClangOptionDocEmitter.cpp

Removed: 




diff  --git a/clang/utils/TableGen/ClangOptionDocEmitter.cpp 
b/clang/utils/TableGen/ClangOptionDocEmitter.cpp
index 75f5d057c33a57..3e56c02a0b3f59 100644
--- a/clang/utils/TableGen/ClangOptionDocEmitter.cpp
+++ b/clang/utils/TableGen/ClangOptionDocEmitter.cpp
@@ -31,13 +31,41 @@ struct DocumentedGroup;
 struct Documentation {
   std::vector Groups;
   std::vector Options;
+
+  bool empty() {
+return Groups.empty() && Options.empty();
+  }
 };
 struct DocumentedGroup : Documentation {
   Record *Group;
 };
 
+static bool hasFlag(const Record *Option, StringRef OptionFlag) {
+  for (const Record *Flag : Option->getValueAsListOfDefs("Flags"))
+if (Flag->getName() == OptionFlag)
+  return true;
+  if (const DefInit *DI = dyn_cast(Option->getValueInit("Group")))
+for (const Record *Flag : DI->getDef()->getValueAsListOfDefs("Flags"))
+  if (Flag->getName() == OptionFlag)
+return true;
+  return false;
+}
+
+static bool isOptionVisible(const Record *Option, const Record *DocInfo) {
+  for (StringRef Exclusion : DocInfo->getValueAsListOfStrings("ExcludedFlags"))
+if (hasFlag(Option, Exclusion))
+  return false;
+  if (!DocInfo->getValue("IncludedFlags"))
+return true;
+  for (StringRef Inclusion : DocInfo->getValueAsListOfStrings("IncludedFlags"))
+if (hasFlag(Option, Inclusion))
+  return true;
+  return false;
+}
+
 // Reorganize the records into a suitable form for emitting documentation.
-Documentation extractDocumentation(RecordKeeper &Records) {
+Documentation extractDocumentation(RecordKeeper &Records,
+   const Record *DocInfo) {
   Documentation Result;
 
   // Build the tree of groups. The root in the tree is the fake option group
@@ -124,12 +152,15 @@ Documentation extractDocumentation(RecordKeeper &Records) 
{
   D.Groups.back().Group = G;
   Documentation &Base = D.Groups.back();
   Base = DocumentationForGroup(G);
+  if (Base.empty())
+D.Groups.pop_back();
 }
 
 auto &Options = OptionsInGroup[R];
 llvm::sort(Options, CompareByName);
 for (Record *O : Options)
-  D.Options.push_back(DocumentationForOption(O));
+  if (isOptionVisible(O, DocInfo))
+D.Options.push_back(DocumentationForOption(O));
 
 return D;
   };
@@ -161,44 +192,6 @@ unsigned getNumArgsForKind(Record *OptionKind, const 
Record *Option) {
 .Default(0);
 }
 
-bool hasFlag(const Record *OptionOrGroup, StringRef OptionFlag) {
-  for (const Record *Flag : OptionOrGroup->getValueAsListOfDefs("Flags"))
-if (Flag->getName() == OptionFlag)
-  return true;
-  return false;
-}
-
-bool isIncluded(const Record *OptionOrGroup, const Record *DocInfo) {
-  assert(DocInfo->getValue("IncludedFlags") && "Missing includeFlags");
-  for (StringRef Inclusion : DocInfo->getValueAsListOfStrings("IncludedFlags"))
-if (hasFlag(OptionOrGroup, Inclusion))
-  return true;
-  return false;
-}
-
-bool isGroupIncluded(const DocumentedGroup &Group, const Record *DocInfo) {
-  if (isIncluded(Group.Group, DocInfo))
-return true;
-  for (auto &O : Group.Options)
-if (isIncluded(O.Option, DocInfo))
-  return true;
-  for (auto &G : Group.Groups) {
-if (isIncluded(G.Group, DocInfo))
-  return true;
-if (isGroupIncluded(G, DocInfo))
-  return true;
-  }
-  return false;
-}
-
-bool isExcluded(const Record *OptionOrGroup, const Record *DocInfo) {
-  // FIXME: Provide a flag to specify the set of exclusions.
-  for (StringRef Exclusion : DocInfo->getValueAsListOfStrings("ExcludedFlags"))
-if (hasFlag(OptionOrGroup, Exclusion))
-  return true;
-  return false;
-}
-
 std::string escapeRST(StringRef Str) {
   std::string Out;
   for (auto K : Str) {
@@ -319,16 +312,13 @@ void forEachOptionName(const DocumentedOption &Option, 
const Record *DocInfo,
   F(Option.Option);
 
   for (auto *Alias : Option.Aliases)
-if (!isExcluded(Alias, DocInfo) && canSphinxCopeWithOption(Option.Option))
+if (isOptionVisible(Alias, DocInfo) &&
+canSphinxCopeWithOption(Option.Option))
  

[PATCH] D157146: [Clang][Docs] Consolidate option hiding in ClangOptionDocEmitter

[Justin Bogner 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 rGce16c3cf30f5: [Clang][Docs] Consolidate option hiding in 
ClangOptionDocEmitter (authored by bogner).

Changed prior to commit:
  https://reviews.llvm.org/D157146?vs=547371&id=549805#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157146

Files:
  clang/utils/TableGen/ClangOptionDocEmitter.cpp

Index: clang/utils/TableGen/ClangOptionDocEmitter.cpp
===
--- clang/utils/TableGen/ClangOptionDocEmitter.cpp
+++ clang/utils/TableGen/ClangOptionDocEmitter.cpp
@@ -31,13 +31,41 @@
 struct Documentation {
   std::vector Groups;
   std::vector Options;
+
+  bool empty() {
+return Groups.empty() && Options.empty();
+  }
 };
 struct DocumentedGroup : Documentation {
   Record *Group;
 };
 
+static bool hasFlag(const Record *Option, StringRef OptionFlag) {
+  for (const Record *Flag : Option->getValueAsListOfDefs("Flags"))
+if (Flag->getName() == OptionFlag)
+  return true;
+  if (const DefInit *DI = dyn_cast(Option->getValueInit("Group")))
+for (const Record *Flag : DI->getDef()->getValueAsListOfDefs("Flags"))
+  if (Flag->getName() == OptionFlag)
+return true;
+  return false;
+}
+
+static bool isOptionVisible(const Record *Option, const Record *DocInfo) {
+  for (StringRef Exclusion : DocInfo->getValueAsListOfStrings("ExcludedFlags"))
+if (hasFlag(Option, Exclusion))
+  return false;
+  if (!DocInfo->getValue("IncludedFlags"))
+return true;
+  for (StringRef Inclusion : DocInfo->getValueAsListOfStrings("IncludedFlags"))
+if (hasFlag(Option, Inclusion))
+  return true;
+  return false;
+}
+
 // Reorganize the records into a suitable form for emitting documentation.
-Documentation extractDocumentation(RecordKeeper &Records) {
+Documentation extractDocumentation(RecordKeeper &Records,
+   const Record *DocInfo) {
   Documentation Result;
 
   // Build the tree of groups. The root in the tree is the fake option group
@@ -124,12 +152,15 @@
   D.Groups.back().Group = G;
   Documentation &Base = D.Groups.back();
   Base = DocumentationForGroup(G);
+  if (Base.empty())
+D.Groups.pop_back();
 }
 
 auto &Options = OptionsInGroup[R];
 llvm::sort(Options, CompareByName);
 for (Record *O : Options)
-  D.Options.push_back(DocumentationForOption(O));
+  if (isOptionVisible(O, DocInfo))
+D.Options.push_back(DocumentationForOption(O));
 
 return D;
   };
@@ -161,44 +192,6 @@
 .Default(0);
 }
 
-bool hasFlag(const Record *OptionOrGroup, StringRef OptionFlag) {
-  for (const Record *Flag : OptionOrGroup->getValueAsListOfDefs("Flags"))
-if (Flag->getName() == OptionFlag)
-  return true;
-  return false;
-}
-
-bool isIncluded(const Record *OptionOrGroup, const Record *DocInfo) {
-  assert(DocInfo->getValue("IncludedFlags") && "Missing includeFlags");
-  for (StringRef Inclusion : DocInfo->getValueAsListOfStrings("IncludedFlags"))
-if (hasFlag(OptionOrGroup, Inclusion))
-  return true;
-  return false;
-}
-
-bool isGroupIncluded(const DocumentedGroup &Group, const Record *DocInfo) {
-  if (isIncluded(Group.Group, DocInfo))
-return true;
-  for (auto &O : Group.Options)
-if (isIncluded(O.Option, DocInfo))
-  return true;
-  for (auto &G : Group.Groups) {
-if (isIncluded(G.Group, DocInfo))
-  return true;
-if (isGroupIncluded(G, DocInfo))
-  return true;
-  }
-  return false;
-}
-
-bool isExcluded(const Record *OptionOrGroup, const Record *DocInfo) {
-  // FIXME: Provide a flag to specify the set of exclusions.
-  for (StringRef Exclusion : DocInfo->getValueAsListOfStrings("ExcludedFlags"))
-if (hasFlag(OptionOrGroup, Exclusion))
-  return true;
-  return false;
-}
-
 std::string escapeRST(StringRef Str) {
   std::string Out;
   for (auto K : Str) {
@@ -319,16 +312,13 @@
   F(Option.Option);
 
   for (auto *Alias : Option.Aliases)
-if (!isExcluded(Alias, DocInfo) && canSphinxCopeWithOption(Option.Option))
+if (isOptionVisible(Alias, DocInfo) &&
+canSphinxCopeWithOption(Option.Option))
   F(Alias);
 }
 
 void emitOption(const DocumentedOption &Option, const Record *DocInfo,
 raw_ostream &OS) {
-  if (isExcluded(Option.Option, DocInfo))
-return;
-  if (DocInfo->getValue("IncludedFlags") && !isIncluded(Option.Option, DocInfo))
-return;
   if (Option.Option->getValueAsDef("Kind")->getName() == "KIND_UNKNOWN" ||
   Option.Option->getValueAsDef("Kind")->getName() == "KIND_INPUT")
 return;
@@ -401,12 +391,6 @@
 
 void emitGroup(int Depth, const DocumentedGroup &Group, const Record *DocInfo,
raw_ostream &OS) {
-  if (isExcluded(Group.Group, DocInfo))
-return;
-
-  if (DocInfo->getValue("Inc

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Haven't looked at everything yet, but wanted to mention one thing I noticed.




Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:91
+
+// Local variables declared inside of the selected lambda cannot go out of
+// scope. The DeclRefExprs that are important are the variables captured 
and

Looking at 
[RecursiveASTVisitor::TraverseLambdaExpr](https://searchfox.org/llvm/rev/094539cfcb46f55824402565e5c18580df689a67/clang/include/clang/AST/RecursiveASTVisitor.h#2652-2691),
 there are a few things it traverses in addition to the lambda's parameters and 
body (which we are saying are ok to skip) and the lambda's captures (which we 
are traversing).

For example, the lambda may have an explicit result type which references a 
variable in scope:

```
int main() {
  if (int a = 1)
if ([[ []() -> decltype(a){ return 1; } ]] () == 4)
  a = a + 1;
}

```

Here, extracting the lambda succeeds, and the reference to `a` in `decltype(a)` 
becomes dangling.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:182-184
+if (InsertionPoint->Parent->ASTNode.get() != nullptr) {
+  return false;
+}

5chmidti wrote:
> This is supposed to stop the following invalid code from happening:
> ```
> void foo() {
>   int placeholder = 42;
>   [](int x = placeholder {};
>   extern void bar(int x = placeholder);
> }
> ```
> 
> clangd does not seem to support extracting from the initializers of defaulted 
> parameters, should I keep the condition as is, or should I do something 
> different here? It is legal to have default arguments in global scope 
> (examples below).
> 
> The following insertions could exist (out of scope for this patch):
> ```
> int placeholder = 42;
> void foo() {
>   [](int x = placeholder {};
>   extern void bar(int x = placeholder);
> }
> ```
> ```
> struct X {
>   static inline int placeholder = 42;
>   void foo(int x = placeholder) {}
> };
> ```
> 
> Either way, I'll have to adjust the comment because this is not just to stop 
> default parameter initializers in lambdas from being extracted to a local 
> scope.
I'm having trouble following this comment. Can you give the testcase that would 
produce this invalid code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141757

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

[PATCH] D157837: [flang][driver] Update the visibility of Clang options in Flang

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski created this revision.
awarzynski added a reviewer: bogner.
Herald added a reviewer: sscalpone.
Herald added projects: Flang, All.
awarzynski requested review of this revision.
Herald added subscribers: cfe-commits, wangpc, jplehr, sstefan1, jdoerfert, 
MaskRay.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

Prior to D157151 , there was no mechanism to 
"disable" unsupported Clang
options in Flang. While the "help" text (`flang-new -help`) was indeed
configured not to display such options, the Flang compiler driver would
happily consume such options and only issue a warning when there was no
logic to parse it:

  flang-new -fno-experimental-relative-c++-abi-vtables file.f90
  flang-new: warning: argument unused during compilation: 
'-fno-experimental-relative-c++-abi-vtables'

D157151  introduces visibility flags. In 
particular, all Clang options
gain a visibility flag (`Default`) that can be excluded when in Flang
driver mode. This way the above invocation will lead to:

  flang-new: error: unknown argument 
'-fno-experimental-relative-c++-abi-vtables'; did you mean '-Xclang 
-fno-experimental-relative-c++-abi-vtables'?

This won't work unless all options/flags supported by Flang have their
visibility flags updated accordingly, hence the changes in Options.td.
Moving forward, this change will allow Flang better control over the
supported options.

Depends on D157151 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157837

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/fast_math.f90
  flang/test/Driver/target-cpu-features.f90

Index: flang/test/Driver/target-cpu-features.f90
===
--- flang/test/Driver/target-cpu-features.f90
+++ flang/test/Driver/target-cpu-features.f90
@@ -11,7 +11,7 @@
 ! RUN: | FileCheck %s -check-prefix=CHECK-ARMV9
 
 ! Negative test. ARM cpu with x86 target.
-! RUN: not %flang --target=x86_64-linux-gnu -mcpu=cortex-a57 -c %s -### 2>&1 \
+! RUN: %flang --target=x86_64-linux-gnu -mcpu=cortex-a57 -c %s -### 2>&1 \
 ! RUN: | FileCheck %s -check-prefix=CHECK-NO-A57
 
 ! RUN: %flang --target=x86_64-linux-gnu -march=skylake -c %s -### 2>&1 \
Index: flang/test/Driver/fast_math.f90
===
--- flang/test/Driver/fast_math.f90
+++ flang/test/Driver/fast_math.f90
@@ -70,7 +70,7 @@
 ! UNSUPPORTED: system-windows
 ! UNSUPPORTED: target=powerpc{{.*}}
 ! RUN: %flang -ffast-math -### %s -o %t 2>&1 \
-! RUN:   --target=x86_64-unknown-linux -no-pie --gcc-toolchain="" \
+! RUN:   --target=x86_64-unknown-linux -no-pie \
 ! RUN:   --sysroot=%S/../../../clang/test/Driver/Inputs/basic_linux_tree \
 ! RUN: | FileCheck --check-prefix=CHECK-CRT %s
 ! CHECK-CRT: {{crtbegin.?\.o}}
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -14,224 +14,242 @@
 ! HELP:USAGE: flang
 ! HELP-EMPTY:
 ! HELP-NEXT:OPTIONS:
-! HELP-NEXT: -###   Print (but do not run) the commands to run for this compilation
-! HELP-NEXT: -cpp   Enable predefined and command line preprocessor macros
-! HELP-NEXT: -c Only run preprocess, compile, and assemble steps
-! HELP-NEXT: -D = Define  to  (or 1 if  omitted)
-! HELP-NEXT: -emit-llvm Use the LLVM representation for assembler and object files
-! HELP-NEXT: -E Only run the preprocessor
+! HELP-NEXT: -###Print (but do not run) the commands to run for this compilation
+! HELP-NEXT: -cppEnable predefined and command line preprocessor macros
+! HELP-NEXT: -c  Only run preprocess, compile, and assemble steps
+! HELP-NEXT: -D =  Define  to  (or 1 if  omitted)
+! HELP-NEXT: -emit-llvm  Use the LLVM representation for assembler and object files
+! HELP-NEXT: -E  Only run the preprocessor
 ! HELP-NEXT: -falternative-parameter-statement
-! HELP-NEXT: Enable the old style PARAMETER statement
-! HELP-NEXT: -fapprox-func  Allow certain math function calls to be replaced with an approximately equivalent calculation
-! HELP-NEXT: -fbackslashSpecify that backslash in string introduces an escape character
-! HELP-NEXT: -fcolor-diagnosticsEnable colors in diagnostics
-! HELP-NEXT: -fconvert=  Set endian conversion of data for unformatted files
-! HELP-NEXT: -fdefault-double-8 Set the default double precision kind to an 8 byte wide type
-! HELP-NEXT: -fdefault-integer-8Set the default integer and logical kind to an 8 byte wide type
-! HELP

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D154503#4576481 , @aaron.ballman 
wrote:

> In D154503#4576475 , @kadircet 
> wrote:
>
>> In D154503#4576442 , 
>> @aaron.ballman wrote:
>>
>>> I'm not opposed to a revert if this is causing problems for your 
>>> downstream, but be sure to also remove the changes from the 17.x branch if 
>>> you go this route rather than fixing forward.
>>
>> Thanks! I didn't notice it made into the release branch. I'd wait for 
>> author's comment on this one, as I guess we can cherry-pick just the 
>> fix-forward if it turns out to be small and safe. Otherwise I am more than 
>> happy to request the cherry-pick for the revert.
>
> SGTM!

Requested cherry-pick in https://github.com/llvm/llvm-project/issues/64670


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154503

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

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

In D157151#4582441 , @bogner wrote:

> This is a little bit complicated by the fact that the Option library is 
> already partially extracted out of clang (and used for other drivers, like 
> lld and lldb). The "Default" visibility is defined in llvm's Option library, 
> so calling it something like "Clang" would be pretty confusing for users that 
> aren't the clang Driver library.

Ah, I see. I guess `clangDriver` is a bit unique in the sense that there's 
quite a few drivers that rely on its Options.td:

- `clang,` `clang -cc1`, `clang -cc1as`, `clang-cl`,
- `flang-new`, `flang-new -fc1`.

> I suppose one option would be to throw something like `using ClangDriver = 
> llvm::Driver::Default;` in Options.h inside the `clang::driver::options` 
> namespace, and then using the alias in Options.td. Would that help?

That would make sense to me, yes. Otherwise the meaning of `Default` is 
unclear. These things tend to be obvious to folks familiar with the 
implementation. However, Options.td would normally be edited/updated by people 
less familiar with the fine details and more concerned about their favorite 
option and how it's implemented in their favorite tool.

> `Default` options are either options that don't specify `Vis` at all or 
> explicitly mention `Default`. They are not visible in `flang-new` after this 
> change unless they also specify `FlangOption`.

That's not quite true ATM. Although not used,  the following C++ option _is 
visible_ in `flang-new`:

  flang-new -fno-experimental-relative-c++-abi-vtables file.f90
  flang-new: warning: argument unused during compilation: 
'-fno-experimental-relative-c++-abi-vtables'

This can be tweaked in `getOptionVisibilityMask` (extracted from this patch):

  llvm::opt::Visibility
  Driver::getOptionVisibilityMask(bool UseDriverMode) const {
if (!UseDriverMode)
  return llvm::opt::Visibility(llvm::opt::Default);
if (IsCLMode())
  return llvm::opt::Visibility(options::CLOption);
if (IsDXCMode())
  return llvm::opt::Visibility(options::DXCOption);
if (IsFlangMode()) {
  // vvv HERE vvv
  // TODO: Does flang really want *all* of the clang driver options?
  // We probably need to annotate more specifically.
  return llvm::opt::Visibility(llvm::opt::Default | options::FlangOption);
}
return llvm::opt::Visibility(llvm::opt::Default);
  }

Now, prior to this change I couldn't find a way to disable unsupported Clang 
options in Flang. With this patch,

- all Clang options gain a visibility flag (atm that's `Default`),
- that flag can be used to disable options unsupported in Flang.

For this to work, all options supported by Flang need to have their visibility 
flags set accordingly. That's the goal, but I can see that we have missed quite 
a few options over the time (*). Updated here: https://reviews.llvm.org/D157837.

(*) There was no mechanism to enforce that. This is now changing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157151

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

[PATCH] D157829: [clang-tidy] Added a new option to lambda-function-name to ignore warnings in macro expansion

[Piotr Zegar via Phabricator via cfe-commits]

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

Missing storeOptions metod, everyting else looks +- fine.




Comment at: clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp:81
 Result.SourceManager->getImmediateExpansionRange(E->getLocation());
-if (SuppressMacroExpansions.find(ER.getAsRange()) !=
-SuppressMacroExpansions.end()) {
+if (IgnoreMacros || SuppressMacroExpansions.find(ER.getAsRange()) !=
+SuppressMacroExpansions.end()) {

Better would be to check IgnoreMacros before we read ER, in separate if 



Comment at: clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h:38
+  : ClangTidyCheck(Name, Context),
+IgnoreMacros(Options.get("IgnoreMacros", DefaultIgnoreMacros)) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;

use getLocalOrGlobal, in future it may become more usefull



Comment at: clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h:43
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:

Missing storeOptions metod


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157829

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

[PATCH] D157239: [clang-tidy] Implement bugprone-incorrect-enable-if

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL 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/D157239/new/

https://reviews.llvm.org/D157239

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

[PATCH] D157838: [clang-repl] Disambiguate declarations with private typedefs

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: v.g.vassilev, rsmith.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Member functions and static variable definitions may use typedefs that
are private in the global context, but fine in the class context.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157838

Files:
  clang/lib/Parse/ParseTentative.cpp
  clang/test/Interpreter/disambiguate-decl-stmt.cpp


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions -std=c++20 %s
 // RUN: %clang_cc1 -fsyntax-only -DMS -fms-extensions -verify 
-fincremental-extensions -std=c++20 %s
 
-// expected-no-diagnostics
-
 extern "C" int printf(const char*,...);
 
 // Decls which are hard to disambiguate
@@ -43,6 +41,37 @@
 
 // Ctors
 
+// Private typedefs / using declarations
+class PrivateUsingMember { using T = int; T f(); };
+PrivateUsingMember::T PrivateUsingMember::f() { return 0; }
+
+class PrivateUsingVar { using T = int; static T i; };
+PrivateUsingVar::T PrivateUsingVar::i = 42;
+
+// The same with namespaces
+namespace PrivateUsingNamespace { class Member { using T = int; T f(); }; }
+PrivateUsingNamespace::Member::T PrivateUsingNamespace::Member::f() { return 
0; }
+
+namespace PrivateUsingNamespace { class Var { using T = int; static T i; }; }
+PrivateUsingNamespace::Var::T PrivateUsingNamespace::Var::i = 42;
+
+// The same with friend declarations
+class PrivateUsingFriendMember;
+class PrivateUsingFriendVar;
+class PrivateUsingFriend { friend class PrivateUsingFriendMember; friend class 
PrivateUsingFriendVar; using T = int; };
+class PrivateUsingFriendMember { PrivateUsingFriend::T f(); };
+PrivateUsingFriend::T PrivateUsingFriendMember::f() { return 0; }
+
+class PrivateUsingFriendVar { static PrivateUsingFriend::T i; };
+PrivateUsingFriend::T PrivateUsingFriendVar::i = 42;
+
+// The following should still diagnose (inspired by PR13642)
+// FIXME: Should not be diagnosed twice!
+class PR13642 { class Inner { public: static int i; }; };
+// expected-note@-1 2 {{implicitly declared private here}}
+PR13642::Inner::i = 5;
+// expected-error@-1 2 {{'Inner' is a private member of 'PR13642'}}
+
 // Deduction guide
 template struct A { A(); A(T); };
 A() -> A;
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -82,6 +82,17 @@
   isDeductionGuide,
   DeclSpec::FriendSpecified::No))
 return true;
+} else if (SS.isNotEmpty()) {
+  // If the scope is not empty, it could alternatively be something 
like
+  // a typedef or using declaration. That declaration might be private
+  // in the global context, which would be diagnosed by calling into
+  // isCXXSimpleDeclaration, but may actually be fine in the context of
+  // member functions and static variable definitions. Check if the 
next
+  // token is also an identifier and assume a declaration.
+  // We cannot check if the scopes match because the declarations could
+  // involve namespaces and friend declarations.
+  if (NextToken().is(tok::identifier))
+return true;
 }
 break;
   }


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions -std=c++20 %s
 // RUN: %clang_cc1 -fsyntax-only -DMS -fms-extensions -verify -fincremental-extensions -std=c++20 %s
 
-// expected-no-diagnostics
-
 extern "C" int printf(const char*,...);
 
 // Decls which are hard to disambiguate
@@ -43,6 +41,37 @@
 
 // Ctors
 
+// Private typedefs / using declarations
+class PrivateUsingMember { using T = int; T f(); };
+PrivateUsingMember::T PrivateUsingMember::f() { return 0; }
+
+class PrivateUsingVar { using T = int; static T i; };
+PrivateUsingVar::T PrivateUsingVar::i = 42;
+
+// The same with namespaces
+namespace PrivateUsingNamespace { class Member { using T = int; T f(); }; }
+PrivateUsingNamespace::Member::T PrivateUsingNamespace::Member::f() { return 0; }
+
+namespace PrivateUsingNamespace { class Var { using T = int; static T i; }; }
+PrivateUsingNamespace::Var::T PrivateUsingNamespace::Var::i = 42;
+
+// The same with friend declarations
+class PrivateUsingFriendMember;
+class PrivateUsingFriendVar;
+class PrivateU

[clang] ba475a4 - [clang-repl] Disambiguate global namespace identifiers

[Jonas Hahnfeld via cfe-commits]

Author: Jonas Hahnfeld
Date: 2023-08-14T10:11:27+02:00
New Revision: ba475a4a3440a21e5de6a39a16215836f2614f04

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

LOG: [clang-repl] Disambiguate global namespace identifiers

A double colon starts an identifier name in the global namespace and
must be tentatively parsed as such.

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

Added: 


Modified: 
clang/lib/Parse/ParseTentative.cpp
clang/test/Interpreter/disambiguate-decl-stmt.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseTentative.cpp 
b/clang/lib/Parse/ParseTentative.cpp
index b7c83bbeb82ee7..66433705250010 100644
--- a/clang/lib/Parse/ParseTentative.cpp
+++ b/clang/lib/Parse/ParseTentative.cpp
@@ -62,6 +62,7 @@ bool Parser::isCXXDeclarationStatement(
   case tok::kw_static_assert:
   case tok::kw__Static_assert:
 return true;
+  case tok::coloncolon:
   case tok::identifier: {
 if (DisambiguatingWithExpression) {
   RevertingTentativeParsingAction TPA(*this);

diff  --git a/clang/test/Interpreter/disambiguate-decl-stmt.cpp 
b/clang/test/Interpreter/disambiguate-decl-stmt.cpp
index 4553a560ee5324..cbc456da6eb512 100644
--- a/clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ b/clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -38,6 +38,10 @@ struct Dtor1 {~Dtor1();};
 Dtor1::~Dtor1() { printf("Dtor1\n"); }
 Dtor1 d1;
 
+struct Dtor2 { ~Dtor2(); };
+::Dtor2::~Dtor2() { printf("Dtor2\n"); }
+Dtor2 d2;
+
 struct ANestedDtor { struct A1 { struct A2 { ~A2(); }; }; };
 ANestedDtor::A1::A2::~A2() { printf("Dtor A::A1::A2::~A2\n"); }
 



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

[PATCH] D157480: [clang-repl] Disambiguate global namespace identifiers

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGba475a4a3440: [clang-repl] Disambiguate global namespace 
identifiers (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157480

Files:
  clang/lib/Parse/ParseTentative.cpp
  clang/test/Interpreter/disambiguate-decl-stmt.cpp


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -38,6 +38,10 @@
 Dtor1::~Dtor1() { printf("Dtor1\n"); }
 Dtor1 d1;
 
+struct Dtor2 { ~Dtor2(); };
+::Dtor2::~Dtor2() { printf("Dtor2\n"); }
+Dtor2 d2;
+
 struct ANestedDtor { struct A1 { struct A2 { ~A2(); }; }; };
 ANestedDtor::A1::A2::~A2() { printf("Dtor A::A1::A2::~A2\n"); }
 
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -62,6 +62,7 @@
   case tok::kw_static_assert:
   case tok::kw__Static_assert:
 return true;
+  case tok::coloncolon:
   case tok::identifier: {
 if (DisambiguatingWithExpression) {
   RevertingTentativeParsingAction TPA(*this);


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -38,6 +38,10 @@
 Dtor1::~Dtor1() { printf("Dtor1\n"); }
 Dtor1 d1;
 
+struct Dtor2 { ~Dtor2(); };
+::Dtor2::~Dtor2() { printf("Dtor2\n"); }
+Dtor2 d2;
+
 struct ANestedDtor { struct A1 { struct A2 { ~A2(); }; }; };
 ANestedDtor::A1::A2::~A2() { printf("Dtor A::A1::A2::~A2\n"); }
 
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -62,6 +62,7 @@
   case tok::kw_static_assert:
   case tok::kw__Static_assert:
 return true;
+  case tok::coloncolon:
   case tok::identifier: {
 if (DisambiguatingWithExpression) {
   RevertingTentativeParsingAction TPA(*this);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157497: feat: Migrate isArch16Bit

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

Is there some separate discussion thread or proposal about this refactoring and 
its motivations?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157497

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

[PATCH] D157837: [flang][driver] Update the visibility of Clang options in Flang

[Tom Eccles via Phabricator via cfe-commits]

tblah added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:6472-6473
   if (IsFlangMode()) {
 // TODO: Does flang really want *all* of the clang driver options?
 // We probably need to annotate more specifically.
+return llvm::opt::Visibility(options::FlangOption);

I presume this TODO can go now?



Comment at: flang/test/Driver/target-cpu-features.f90:14
 ! Negative test. ARM cpu with x86 target.
-! RUN: not %flang --target=x86_64-linux-gnu -mcpu=cortex-a57 -c %s -### 2>&1 \
+! RUN: %flang --target=x86_64-linux-gnu -mcpu=cortex-a57 -c %s -### 2>&1 \
 ! RUN: | FileCheck %s -check-prefix=CHECK-NO-A57

Why doesn't this fail now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157837

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

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This mostly looks good.

My main concern is adding code to the parser to suppress errors when code 
completing: this is unlikely to be the only such place. Existing consumers seem 
happy to ignore errors, if this doesn't work for clang-repl it'd be good to 
understand why.




Comment at: clang/include/clang/Frontend/ASTUnit.h:891
   ///
+  /// \param Act A SyntaxOnlyAction performs actions on the syntax.
+  ///

This comment doesn't really say anything beyond echoing the name/type, either 
remove it or add some separate explanation?

e.g. "If Act is specified, it will be used to parse the file. This allows 
customizing the parse by overriding FrontendAction's lifecycle methods."



Comment at: clang/include/clang/Frontend/ASTUnit.h:901
+SmallVectorImpl &OwnedBuffers,
+std::function 
AfterBeginSourceFile = [](CompilerInstance& CI) -> void {});
 

capfredf wrote:
> @v.g.vassilev  Not sure if this is the right solution or idiom to extend this 
> method.  
> 
> we can make this high order function more general, like 
> `std::unique_ptr<()>`
There's no need for this to be a SyntaxOnlyAction, you can take FrontendAction 
here.

I don't think there's any need to provide a factory, it's safe to assume it 
will always create one action. (We have FrontendActionFactory in tooling, and 
it's a pain.)



Comment at: clang/include/clang/Interpreter/InterpreterCodeCompletion.h:30
+std::vector
+ConvertToCodeCompleteStrings(const std::vector &Results);
+} // namespace clang

nit: lowercase initial letter for functions



Comment at: clang/include/clang/Interpreter/InterpreterCodeCompletion.h:30
+std::vector
+ConvertToCodeCompleteStrings(const std::vector &Results);
+} // namespace clang

sammccall wrote:
> nit: lowercase initial letter for functions
you might consider `vector` instead.

this signature doesn't provide any way to return owned names, and so requires 
that the results are already allocated somewhere as contiguous strings.

This will prevents completing names like `operator int`, as well as some 
non-names that might prove useful.

This is an interactive operation, copying the strings is unlikely to add 
significant latency on human scale.



Comment at: clang/include/clang/Sema/CodeCompleteConsumer.h:342
+/// Code completion at a top level in a REPL session.
+CCC_ReplTopLevel,
   };

v.g.vassilev wrote:
> 
I don't think this name fits with the others, it describes the client rather 
than the grammatical/semantic context.

I would suggest maybe `CCC_TopLevelOrExpression`?



Comment at: clang/lib/Interpreter/InterpreterCodeCompletion.cpp:72
+  break;
+default:
+  break;

(up to you, but default between cases seems surprising)



Comment at: clang/lib/Parse/ParseDecl.cpp:6648
 if (Tok.is(tok::l_paren)) {
+  if (PP.isIncrementalProcessingEnabled() &&
+  NextToken().is(tok::code_completion)) {

AIUI, all code completion actions should be ignoring errors, and we should 
always be happy to cut off parsing and give up as soon as the completion token 
is reached.

So why do we need special handling for incremental processing here?

(i.e. what behavior do non-repl users see here - if it's correct can clang-repl 
do the same, if it's wrong why don't we fix both?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154382

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

[PATCH] D157150: [Driver] Update BoolOption to handle Visibility. NFC

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 549853.
bogner added a comment.

Use "ClangOption" rather than "Default" in Options.td, as per the conversation 
in https://reviews.llvm.org/D157151


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157150

Files:
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td

Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -79,6 +79,23 @@
 // target will lead to an err_drv_unsupported_opt_for_target error.
 def TargetSpecific : OptionFlag;
 
+// Indicates that this warning is ignored, but accepted with a warning for
+// GCC compatibility.
+class IgnoredGCCCompat : Flags<[HelpHidden]> {}
+
+class TargetSpecific : Flags<[TargetSpecific]> {}
+
+/
+// Visibility
+
+// We prefer the name "ClangOption" here rather than "Default" to make
+// it clear that these options will be visible in the clang driver (as
+// opposed to clang -cc1, the CL driver, or the flang driver).
+defvar ClangOption = Default;
+
+/
+// Docs
+
 // A short name to show in documentation. The name will be interpreted as rST.
 class DocName { string DocName = name; }
 
@@ -89,12 +106,6 @@
 // documentation.
 class DocFlatten { bit DocFlatten = 1; }
 
-// Indicates that this warning is ignored, but accepted with a warning for
-// GCC compatibility.
-class IgnoredGCCCompat : Flags<[HelpHidden]> {}
-
-class TargetSpecific : Flags<[TargetSpecific]> {}
-
 /
 // Groups
 
@@ -381,7 +392,8 @@
 
 // Definition of single command line flag. This is an implementation detail, use
 // SetTrueBy or SetFalseBy instead.
-class FlagDef option_flags,
+class FlagDef option_flags, list option_vis,
   string help, list implied_by_expressions = []> {
   // The polarity. Besides spelling, this also decides whether the TableGen
   // record will be prefixed with "no_".
@@ -390,9 +402,12 @@
   // The value assigned to key path when the flag is present on command line.
   bit Value = value;
 
-  // OptionFlags that control visibility of the flag in different tools.
+  // OptionFlags in different tools.
   list OptionFlags = option_flags;
 
+  // OptionVisibility flags for different tools.
+  list OptionVis = option_vis;
+
   // The help text associated with the flag.
   string Help = help;
 
@@ -401,8 +416,11 @@
 }
 
 // Additional information to be appended to both positive and negative flag.
-class BothFlags option_flags, string help = ""> {
+class BothFlags option_flags,
+list option_vis = [ClangOption],
+string help = ""> {
   list OptionFlags = option_flags;
+  list OptionVis = option_vis;
   string Help = help;
 }
 
@@ -411,23 +429,26 @@
   FlagDef Result
 = FlagDef;
 }
 
 // Definition of the command line flag with positive spelling, e.g. "-ffoo".
-class PosFlag flags = [], string help = "",
-  list implied_by_expressions = []>
-  : FlagDef {}
+class PosFlag flags = [], list vis = [],
+  string help = "", list implied_by_expressions = []>
+  : FlagDef {}
 
 // Definition of the command line flag with negative spelling, e.g. "-fno-foo".
-class NegFlag flags = [], string help = "",
-  list implied_by_expressions = []>
-  : FlagDef {}
+class NegFlag flags = [], list vis = [],
+  string help = "", list implied_by_expressions = []>
+  : FlagDef {}
 
 // Expanded FlagDef that's convenient for creation of TableGen records.
 class FlagDefExpanded
-  : FlagDef {
+  : FlagDef {
   // Name of the TableGen record.
   string RecordName = prefix # !if(flag.Polarity, "", "no_") # name;
 
@@ -445,7 +466,8 @@
 class MarshalledFlagRec
-  : Flag<["-"], flag.Spelling>, Flags, HelpText,
+  : Flag<["-"], flag.Spelling>, Flags, Vis,
+HelpText,
 MarshallingInfoBooleanFlag,
 ImpliedByAnyOf {}
@@ -459,7 +481,7 @@
 multiclass BoolOption> {
+  BothFlags suffix = BothFlags<[]>> {
   defvar flag1 = FlagDefExpanded.Result, prefix,
  NAME, spelling_base>;
 
@@ -490,7 +512,7 @@
 /// CompilerInvocation.
 multiclass BoolFOption> {
+   BothFlags both = BothFlags<[]>> {
   defm NAME : BoolOption<"f", flag_base, kpm, default, flag1, flag2, both>,
   Group;
 }
@@ -501,7 +523,7 @@
 // CompilerInvocation.
 multiclass BoolGOption> {
+   BothFlags both = BothFlags<[]>> {
   defm NAME : BoolOption<"g", flag_base, kpm, default, flag1, flag2, both>,
   Group;
 }
@@ -509,7 +531,7 @@
 // Works like BoolOption except without marshalling
 multiclass BoolOptionWithoutMarshalling> {
+BothFlags suffix = BothFlags<[]>> {
   defvar flag1 = FlagDefExpanded.Result, prefix,
  NAME, spelling_base>;
 
@@ -531,11 +553,11 @@

[PATCH] D157855: [clang][ExprConstant] Improve error message of compound assignment against uninitialized object

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet created this revision.
hazohelet added reviewers: aaron.ballman, tbaeder, cjdb.
Herald added a project: All.
hazohelet requested review of this revision.
Herald added a project: clang.

BEFORE this patch, compound assignment operator against uninitialized object 
such as `uninit += 1` was diagnosed as `subexpression not valid`
This patch clarifies the reason for the error by saying that `uninit` is an 
uninitialized object.

Fixes https://github.com/llvm/llvm-project/issues/51536


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157855

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx2a.cpp


Index: clang/test/SemaCXX/constant-expression-cxx2a.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1492,3 +1492,33 @@
 class D : B{}; // expected-error {{deleted function '~D' cannot override a 
non-deleted function}}
 // expected-note@-1 {{destructor of 'D' is implicitly deleted because base 
class 'B' has an inaccessible destructor}}
 }
+
+namespace UninitCompoundAssign {
+constexpr int scalar(int a) {
+  int sum;
+  sum += a; // expected-note {{read of uninitialized object}};
+  return 0;
+}
+static_assert(scalar(3)); // expected-error {{constant expression}} \
+  // expected-note {{in call to 'scalar(3)'}}
+
+constexpr int array(int a) {
+  int arr[3];
+  arr[1] += a; // expected-note {{read of uninitialized object}};
+  return 0;
+}
+static_assert(array(3)); // expected-error {{constant expression}} \
+ // expected-note {{in call to 'array(3)'}}
+
+struct Foo {
+  int val;
+  constexpr Foo() {}
+};
+constexpr int field(int a) {
+  Foo f;
+  f.val += a; // expected-note {{read of uninitialized object}};
+  return 0;
+}
+static_assert(field(3)); // expected-error {{constant expression}} \
+ // expected-note {{in call to 'field(3)'}}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4442,6 +4442,10 @@
   return foundPointer(Subobj, SubobjType);
 case APValue::Vector:
   return foundVector(Subobj, SubobjType);
+case APValue::Indeterminate:
+  Info.FFDiag(E, diag::note_constexpr_access_uninit)
+  << /*read of*/ 0 << /*uninitialized object*/ 1;
+  return false;
 default:
   // FIXME: can this happen?
   Info.FFDiag(E);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -129,6 +129,9 @@
   of a base class is not called in the constructor of its derived class.
 - Clang no longer emits ``-Wmissing-variable-declarations`` for variables 
declared
   with the ``register`` storage class.
+- Clang constexpr evaluator now diagnoses compound assignment operators against
+  uninitialized variables as a read of uninitialized object.
+  (`#51536 _`)
 
 Bug Fixes in This Version
 -


Index: clang/test/SemaCXX/constant-expression-cxx2a.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1492,3 +1492,33 @@
 class D : B{}; // expected-error {{deleted function '~D' cannot override a non-deleted function}}
 // expected-note@-1 {{destructor of 'D' is implicitly deleted because base class 'B' has an inaccessible destructor}}
 }
+
+namespace UninitCompoundAssign {
+constexpr int scalar(int a) {
+  int sum;
+  sum += a; // expected-note {{read of uninitialized object}};
+  return 0;
+}
+static_assert(scalar(3)); // expected-error {{constant expression}} \
+  // expected-note {{in call to 'scalar(3)'}}
+
+constexpr int array(int a) {
+  int arr[3];
+  arr[1] += a; // expected-note {{read of uninitialized object}};
+  return 0;
+}
+static_assert(array(3)); // expected-error {{constant expression}} \
+ // expected-note {{in call to 'array(3)'}}
+
+struct Foo {
+  int val;
+  constexpr Foo() {}
+};
+constexpr int field(int a) {
+  Foo f;
+  f.val += a; // expected-note {{read of uninitialized object}};
+  return 0;
+}
+static_assert(field(3)); // expected-error {{constant expression}} \
+ // expected-note {{in call to 'field(3)'}}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4442,6 +4442,10 @@
   return foundPointer(Subobj, SubobjType);
 case APValue::Vector:
   return foundVector(Subobj, SubobjType);
+case APValue::Indeterminate:
+  

[PATCH] D155773: [llvm][MemoryBuiltins] Add alloca support to getInitialValueOfAllocation

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

@jmciver Thanks for the context. I would recommend you to put up the patch you 
have based on top of this, because it's pretty hard to tell whether this API 
design makes sense without seeing the use. I have some doubts about that, for 
two reasons:

- If I understand correctly, for your use case getInitialValueOfAllocation() 
can't just return a constant anymore, it may have to insert an instruction. 
Additionally, there is no longer a single "initial value", but it may wary 
between different loads from the same allocation. This means that the API is 
going to change to the point that getInitialValueOfAllocation() is no longer 
recognizable (and probably no longer correctly named -- it would be more 
materializeInitialValueOfAllocation()).
- As some of the changes in this patch show, we also have to give 
lifetime.start intrinsics similar treatment to allocas, but this doesn't quite 
fit the current API.

I think supporting allocas in getInitialValueOfAllocation() is perfectly fine, 
but I'm not sure this really brings you closer to what you want to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155773

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

[clang] 00158ae - [clang] Enable constexpr on LZCNT/POPCNT MS extension intrinsics

[Simon Pilgrim via cfe-commits]

Author: Alejandro Aguirre
Date: 2023-08-14T11:33:33+01:00
New Revision: 00158ae236ddfdc7dda5ea7a8a20e3921007ba86

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

LOG: [clang] Enable constexpr on LZCNT/POPCNT MS extension intrinsics

As discussed on #46593 - this enables us to use __lzcnt / __popcnt intrinsics 
inside constexpr code.

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

Added: 


Modified: 
clang/docs/LanguageExtensions.rst
clang/include/clang/Basic/Builtins.def
clang/lib/AST/ExprConstant.cpp
clang/test/CodeGen/ms-intrinsics-other.c

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 386ffa7b48eb93..c6f781dfe35e89 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -5225,12 +5225,18 @@ The following x86-specific intrinsics can be used in 
constant expressions:
 * ``_castf64_u64``
 * ``_castu32_f32``
 * ``_castu64_f64``
+* ``__lzcnt16``
+* ``__lzcnt``
+* ``__lzcnt64``
 * ``_mm_popcnt_u32``
 * ``_mm_popcnt_u64``
 * ``_popcnt32``
 * ``_popcnt64``
 * ``__popcntd``
 * ``__popcntq``
+* ``__popcnt16``
+* ``__popcnt``
+* ``__popcnt64``
 * ``__rolb``
 * ``__rolw``
 * ``__rold``

diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index 83e4259ea037b9..f5374537a0242c 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -1017,12 +1017,12 @@ LANGBUILTIN(__iso_volatile_store16, "vsD*s", "n", 
ALL_MS_LANGUAGES)
 LANGBUILTIN(__iso_volatile_store32, "viD*i", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__iso_volatile_store64, "vLLiD*LLi", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__noop,   "i.",  "n", ALL_MS_LANGUAGES)
-LANGBUILTIN(__lzcnt16, "UsUs","nc", ALL_MS_LANGUAGES)
-LANGBUILTIN(__lzcnt,   "UiUi","nc", ALL_MS_LANGUAGES)
-LANGBUILTIN(__lzcnt64, "UWiUWi",  "nc", ALL_MS_LANGUAGES)
-LANGBUILTIN(__popcnt16, "UsUs",   "nc", ALL_MS_LANGUAGES)
-LANGBUILTIN(__popcnt,   "UiUi",   "nc", ALL_MS_LANGUAGES)
-LANGBUILTIN(__popcnt64, "UWiUWi", "nc", ALL_MS_LANGUAGES)
+LANGBUILTIN(__lzcnt16, "UsUs","ncE", ALL_MS_LANGUAGES)
+LANGBUILTIN(__lzcnt,   "UiUi","ncE", ALL_MS_LANGUAGES)
+LANGBUILTIN(__lzcnt64, "UWiUWi",  "ncE", ALL_MS_LANGUAGES)
+LANGBUILTIN(__popcnt16, "UsUs",   "ncE", ALL_MS_LANGUAGES)
+LANGBUILTIN(__popcnt,   "UiUi",   "ncE", ALL_MS_LANGUAGES)
+LANGBUILTIN(__popcnt64, "UWiUWi", "ncE", ALL_MS_LANGUAGES)
 LANGBUILTIN(_ReturnAddress, "v*", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_rotl8,  "UcUcUc","nE", ALL_MS_LANGUAGES)
 LANGBUILTIN(_rotl16, "UsUsUc","nE", ALL_MS_LANGUAGES)

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index a7e0d5a38698f7..ee2f3b7be1dabd 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12124,11 +12124,21 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const 
CallExpr *E,
   case Builtin::BI__builtin_clz:
   case Builtin::BI__builtin_clzl:
   case Builtin::BI__builtin_clzll:
-  case Builtin::BI__builtin_clzs: {
+  case Builtin::BI__builtin_clzs:
+  case Builtin::BI__lzcnt16: // Microsoft variants of count leading-zeroes
+  case Builtin::BI__lzcnt:
+  case Builtin::BI__lzcnt64: {
 APSInt Val;
 if (!EvaluateInteger(E->getArg(0), Val, Info))
   return false;
-if (!Val)
+
+// When the argument is 0, the result of GCC builtins is undefined, whereas
+// for Microsoft intrinsics, the result is the bit-width of the argument.
+bool ZeroIsUndefined = BuiltinOp != Builtin::BI__lzcnt16 &&
+   BuiltinOp != Builtin::BI__lzcnt &&
+   BuiltinOp != Builtin::BI__lzcnt64;
+
+if (ZeroIsUndefined && !Val)
   return Error(E);
 
 return Success(Val.countl_zero(), E);
@@ -12267,7 +12277,10 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const 
CallExpr *E,
 
   case Builtin::BI__builtin_popcount:
   case Builtin::BI__builtin_popcountl:
-  case Builtin::BI__builtin_popcountll: {
+  case Builtin::BI__builtin_popcountll:
+  case Builtin::BI__popcnt16: // Microsoft variants of popcount
+  case Builtin::BI__popcnt:
+  case Builtin::BI__popcnt64: {
 APSInt Val;
 if (!EvaluateInteger(E->getArg(0), Val, Info))
   return false;

diff  --git a/clang/test/CodeGen/ms-intrinsics-other.c 
b/clang/test/CodeGen/ms-intrinsics-other.c
index 4bb51087b17765..76f54add749669 100644
--- a/clang/test/CodeGen/ms-intrinsics-other.c
+++ b/clang/test/CodeGen/ms-intrinsics-other.c
@@ -14,6 +14,27 @@
 // RUN: -triple armv7--darwin -Oz -emit-llvm %s -o - \
 // RUN: | FileCheck %s --check-prefix=CHECK-ARM
 
+// RUN: %clang_cc1 -x c++ -std=c++11 \
+// RUN: -ffreestanding -fms-extensions 
-Wno-implicit-function-decla

[PATCH] D157420: [clang] Enable constexpr on LZCNT/POPCNT MS extension intrinsics

[Simon Pilgrim via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG00158ae236dd: [clang] Enable constexpr on LZCNT/POPCNT MS 
extension intrinsics (authored by alexguirre, committed by RKSimon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157420

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGen/ms-intrinsics-other.c

Index: clang/test/CodeGen/ms-intrinsics-other.c
===
--- clang/test/CodeGen/ms-intrinsics-other.c
+++ clang/test/CodeGen/ms-intrinsics-other.c
@@ -14,6 +14,27 @@
 // RUN: -triple armv7--darwin -Oz -emit-llvm %s -o - \
 // RUN: | FileCheck %s --check-prefix=CHECK-ARM
 
+// RUN: %clang_cc1 -x c++ -std=c++11 \
+// RUN: -ffreestanding -fms-extensions -Wno-implicit-function-declaration \
+// RUN: -triple x86_64--darwin -Oz -emit-llvm %s -o - \
+// RUN: | FileCheck %s
+// RUN: %clang_cc1 -x c++ -std=c++11 \
+// RUN: -ffreestanding -fms-extensions -Wno-implicit-function-declaration \
+// RUN: -triple x86_64--linux -Oz -emit-llvm %s -o - \
+// RUN: | FileCheck %s
+// RUN: %clang_cc1 -x c++ -std=c++11 \
+// RUN: -ffreestanding -fms-extensions -Wno-implicit-function-declaration \
+// RUN: -triple aarch64--darwin -Oz -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-ARM-ARM64
+// RUN: %clang_cc1 -x c++ -std=c++11 \
+// RUN: -ffreestanding -fms-extensions -Wno-implicit-function-declaration \
+// RUN: -triple aarch64--darwin -Oz -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-ARM
+// RUN: %clang_cc1 -x c++ -std=c++11 \
+// RUN: -ffreestanding -fms-extensions -Wno-implicit-function-declaration \
+// RUN: -triple armv7--darwin -Oz -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-ARM
+
 // LP64 targets use 'long' as 'int' for MS intrinsics (-fms-extensions)
 #ifdef __LP64__
 #define LONG int
@@ -21,6 +42,10 @@
 #define LONG long
 #endif
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 unsigned char test_BitScanForward(unsigned LONG *Index, unsigned LONG Mask) {
   return _BitScanForward(Index, Mask);
 }
@@ -416,3 +441,36 @@
 // CHECK-ARM: ret i32 [[RESULT]]
 // CHECK-ARM: }
 #endif
+
+#ifdef __cplusplus
+}
+#endif
+
+
+// Test constexpr handling.
+#if defined(__cplusplus) && (__cplusplus >= 201103L)
+
+char popcnt16_0[__popcnt16(0x) == 0 ? 1 : -1];
+char popcnt16_1[__popcnt16(0x10F0) == 5 ? 1 : -1];
+
+char popcnt_0[__popcnt(0x) == 0 ? 1 : -1];
+char popcnt_1[__popcnt(0x10F0) == 5 ? 1 : -1];
+
+char popcnt64_0[__popcnt64(0xULL) == 0 ? 1 : -1];
+char popcnt64_1[__popcnt64(0xF0F1ULL) == 9 ? 1 : -1];
+
+#define BITSIZE(x) (sizeof(x) * 8)
+char lzcnt16_0[__lzcnt16(1) == BITSIZE(short) - 1 ? 1 : -1];
+char lzcnt16_1[__lzcnt16(1 << (BITSIZE(short) - 1)) == 0 ? 1 : -1];
+char lzcnt16_2[__lzcnt16(0) == BITSIZE(short) ? 1 : -1];
+
+char lzcnt_0[__lzcnt(1) == BITSIZE(int) - 1 ? 1 : -1];
+char lzcnt_1[__lzcnt(1 << (BITSIZE(int) - 1)) == 0 ? 1 : -1];
+char lzcnt_2[__lzcnt(0) == BITSIZE(int) ? 1 : -1];
+
+char lzcnt64_0[__lzcnt64(1ULL) == BITSIZE(__int64) - 1 ? 1 : -1];
+char lzcnt64_1[__lzcnt64(1ULL << (BITSIZE(__int64) - 1)) == 0 ? 1 : -1];
+char lzcnt64_2[__lzcnt64(0ULL) == BITSIZE(__int64) ? 1 : -1];
+#undef BITSIZE
+
+#endif
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -12124,11 +12124,21 @@
   case Builtin::BI__builtin_clz:
   case Builtin::BI__builtin_clzl:
   case Builtin::BI__builtin_clzll:
-  case Builtin::BI__builtin_clzs: {
+  case Builtin::BI__builtin_clzs:
+  case Builtin::BI__lzcnt16: // Microsoft variants of count leading-zeroes
+  case Builtin::BI__lzcnt:
+  case Builtin::BI__lzcnt64: {
 APSInt Val;
 if (!EvaluateInteger(E->getArg(0), Val, Info))
   return false;
-if (!Val)
+
+// When the argument is 0, the result of GCC builtins is undefined, whereas
+// for Microsoft intrinsics, the result is the bit-width of the argument.
+bool ZeroIsUndefined = BuiltinOp != Builtin::BI__lzcnt16 &&
+   BuiltinOp != Builtin::BI__lzcnt &&
+   BuiltinOp != Builtin::BI__lzcnt64;
+
+if (ZeroIsUndefined && !Val)
   return Error(E);
 
 return Success(Val.countl_zero(), E);
@@ -12267,7 +12277,10 @@
 
   case Builtin::BI__builtin_popcount:
   case Builtin::BI__builtin_popcountl:
-  case Builtin::BI__builtin_popcountll: {
+  case Builtin::BI__builtin_popcountll:
+  case Builtin::BI__popcnt16: // Microsoft variants of popcount
+  case Builtin::BI__popcnt:
+  case Builtin::BI__popcnt64: {
 APSInt Val;
 if (!EvaluateInt

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

[Justin Bogner via Phabricator via cfe-commits]

bogner added a comment.

In D157151#4583904 , @awarzynski 
wrote:

> This can be tweaked in `getOptionVisibilityMask` (extracted from this patch):
>
>   llvm::opt::Visibility
>   Driver::getOptionVisibilityMask(bool UseDriverMode) const {
> if (!UseDriverMode)
>   return llvm::opt::Visibility(llvm::opt::Default);
> if (IsCLMode())
>   return llvm::opt::Visibility(options::CLOption);
> if (IsDXCMode())
>   return llvm::opt::Visibility(options::DXCOption);
> if (IsFlangMode()) {
>   // vvv HERE vvv
>   // TODO: Does flang really want *all* of the clang driver options?
>   // We probably need to annotate more specifically.
>   return llvm::opt::Visibility(llvm::opt::Default | options::FlangOption);
> }
> return llvm::opt::Visibility(llvm::opt::Default);
>   }
>
> Now, prior to this change I couldn't find a way to disable unsupported Clang 
> options in Flang. With this patch,
>
> - all Clang options gain a visibility flag (atm that's `Default`),
> - that flag can be used to disable options unsupported in Flang.
>
> For this to work, all options supported by Flang need to have their 
> visibility flags set accordingly. That's the goal, but I can see that we have 
> missed quite a few options over the time (*). Updated here: 
> https://reviews.llvm.org/D157837.

Ah right. Yes, this is exactly the problem that this patch is trying to fix. 
Glad to see that it seems to be working as intended for flang-new!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157151

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

[PATCH] D157837: [flang][driver] Update the visibility of Clang options in Flang

[Justin Bogner via Phabricator via cfe-commits]

bogner added a comment.

I can't speak to which flags should be present in flang-new or not, but in 
principle this looks great. You'll need to update the patch to use the 
"ClangOption" spelling rather than "Default" of course, as discussed in 
https://reviews.llvm.org/D157151




Comment at: clang/lib/Driver/Driver.cpp:6473
 // TODO: Does flang really want *all* of the clang driver options?
 // We probably need to annotate more specifically.
+return llvm::opt::Visibility(options::FlangOption);

We can remove the TODO comment with this change :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157837

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

[PATCH] D156237: Complete the implementation of P2361 Unevaluated string literals

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 549878.
cor3ntin marked an inline comment as done.
cor3ntin added a comment.

Address Hubert's feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156237

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Parser/MicrosoftExtensions.cpp
  clang/test/Parser/c2x-attributes.c
  clang/test/Parser/cxx-attributes.cpp
  clang/test/Parser/cxx0x-attributes.cpp
  clang/test/Sema/MicrosoftExtensions.c
  clang/test/Sema/annotate-type.c
  clang/test/Sema/annotate.c
  clang/test/Sema/attr-assume.c
  clang/test/Sema/attr-btf_tag.c
  clang/test/Sema/attr-btf_type_tag.c
  clang/test/Sema/attr-capabilities.c
  clang/test/Sema/attr-enforce-tcb-errors.cpp
  clang/test/Sema/attr-enforce-tcb-errors.m
  clang/test/Sema/attr-error.c
  clang/test/Sema/attr-handles.cpp
  clang/test/Sema/attr-section.c
  clang/test/Sema/attr-tls_model.c
  clang/test/Sema/attr-unavailable-message.c
  clang/test/Sema/attr-warning.c
  clang/test/Sema/diagnose_if.c
  clang/test/Sema/enable_if.c
  clang/test/SemaCXX/attr-deprecated-replacement-error.cpp
  clang/test/SemaCXX/attr-no-sanitize.cpp
  clang/test/SemaCXX/attr-section.cpp
  clang/test/SemaCXX/attr-weakref.cpp
  clang/test/SemaCXX/suppress.cpp
  clang/test/SemaObjC/attr-swift_bridge.m
  clang/test/SemaObjC/objc-asm-attribute-neg-test.m
  clang/test/SemaObjC/validate-attr-swift_attr.m
  clang/test/SemaTemplate/attributes.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -115,12 +115,7 @@
  
   Unevaluated strings
   https://wg21.link/P2361R6";>P2361R6
-  
-
-  Clang 17 (Partial)
-  Attributes arguments don't yet parse as unevaluated string literals.
-
-  
+  Clang 18
  
  
   Add @, $, and ` to the basic character set
Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -2297,6 +2297,22 @@
  .Default(false);
 }
 
+static bool isStringLiteralArgument(const Record *Arg) {
+  return !Arg->getSuperClasses().empty() &&
+ llvm::StringSwitch(
+ Arg->getSuperClasses().back().first->getName())
+ .Case("StringArgument", true)
+ .Default(false);
+}
+
+static bool isVariadicStringLiteralArgument(const Record *Arg) {
+  return !Arg->getSuperClasses().empty() &&
+ llvm::StringSwitch(
+ Arg->getSuperClasses().back().first->getName())
+ .Case("VariadicStringArgument", true)
+ .Default(false);
+}
+
 static void emitClangAttrVariadicIdentifierArgList(RecordKeeper &Records,
raw_ostream &OS) {
   OS << "#if defined(CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST)\n";
@@ -2317,6 +2333,34 @@
   OS << "#endif // CLANG_ATTR_VARIADIC_IDENTIFIER_ARG_LIST\n\n";
 }
 
+// Emits the list of arguments that should be parsed as unevaluated string
+// literals for each attribute.
+static void emitClangAttrUnevaluatedStringLiteralList(RecordKeeper &Records,
+  raw_ostream &OS) {
+  OS << "#if defined(CLANG_ATTR_STRING_LITERAL_ARG_LIST)\n";
+  std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
+  for (const auto *Attr : Attrs) {
+std::vector Args = Attr->getValueAsListOfDefs("Args");
+uint32_t Bits = 0;
+assert(Args.size() <= 32 && "unsupported number of arguments in attribute");
+for (uint32_t N = 0; N < Args.size(); ++N) {
+  Bits |= (isStringLiteralArgument(Args[N]) << N);
+  // If we have a variadic string argument, set all the remaining bits to 1
+  if (isVariadicStringLiteralArgument(Args[N])) {
+Bits |= maskTrailingZeros(N);
+break;
+  }
+}
+if (!Bits)
+  continue;
+// All these spellings have at least one string literal has argument.
+forEachUniqueSpelling(*Attr, [&](const FlattenedSpelling &S) {
+  OS << ".Case(\"" << S.name() << "\", " << Bits << ")\n";
+});
+  }
+  OS << "#endif // CLANG_ATTR_STRING_LITERAL_ARG_LIST\n\n";
+}
+
 // Emits the first-argument-is-identifier property for attributes.
 static void emitClangAttrIdentifierArgList(RecordKeeper &Records, raw_ostream &OS) {
   OS << "#if defined(CLANG_ATTR_IDENTIFIER_ARG_LIST)\n";
@@ -4611,6 +4655,7 @@
   emitSourceFileHeader("Parser-related llvm::StringSwitch cases", OS);
   emitClangAttrArgContextList(Records, OS);
   emitClangAttrIdentifierArgList(Records, OS);
+  emitClangAttr

[PATCH] D156237: Complete the implementation of P2361 Unevaluated string literals

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:431-436
+if (Expr.isInvalid()) {
+  SawError = true;
+  break;
+} else {
+  Exprs.push_back(Expr.get());
+}

hubert.reinterpretcast wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > 
> > Oups, sorry @Fznamznon, I'll fix that!
> It seems Aaron's request to move the code out of the `else` was not acted on.
Not sure what happened there, I recall doing that. I guess not, thanks!



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:2345
+uint32_t Bits = 0;
+for (uint32_t N = 0; N < Args.size(); ++N) {
+  Bits |= (isStringLiteralArgument(Args[N]) << N);

hubert.reinterpretcast wrote:
> Do we know that `Args.size()` is less than 32?
I added an assert


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156237

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

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 549879.
victorkingi marked 6 inline comments as done.
victorkingi added a comment.

code refactoring in reference to comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,47 @@
+! This file tests the -Rpass family of flags (-Rpass, -Rpass-missed
+! and -Rpass-analysis)
+! loop-delete isn't enabled at O0 so we use at least O1
+
+! Check that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: optimization-remark.f90:43:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:38:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:40:9: remark: loop not vectorized: call instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
\ No newline at end of file
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Option/Option.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/CommandLine.h"
+#include 
 
 namespace Fortran::frontend {
 
@@ -100,6 +101,61 @@
   llvm_unreachable("Invalid program action!");
 }
 
+// Emit a warning and typo hint for unknown warning opts
+static void emitUnknownDiagWarning(clang::DiagnosticsEngine &diags,
+   clang::diag::Flavor flavor,
+   llvm::StringRef prefix,
+   llvm::StringRef opt) {
+  llvm::StringRef suggestion =
+  clang::DiagnosticIDs::getNearestOption(flavor, opt);
+  diags.Report(clang::diag::warn_unknown_diag_option)
+  << (flavor == clang::diag::Flavor::WarningOrError ? 0 : 1)
+  << (prefix.str() += std::string(opt)) << !suggestion.empty()
+  << (prefix.str() += std::string(suggestion));
+}
+
+void processWarningOptions(clang::DiagnosticsEngine &diags,
+   const clang::DiagnosticOptions &opts,
+   bool reportDiags = true) {
+
+  llvm::SmallVector _diags;
+  const llvm::IntrusiveRefCntPtr diagIDs =
+  diags.getDiagnosticIDs();
+  // We parse the warning options twice.  The first pass sets diagnostic state,
+  // while the second pass reports warnings/errors.  This has the effect that
+  // we follow the more canonical "last option wins" paradigm when there are
+  // conflicting options.
+  for (unsigned report = 0, reportEnd = 2; report != reportEnd; ++report) {
+bool setDiagnostic = (report == 0);
+
+// If we've set the diagnostic state and are not reporting diagnostics then
+// we're done.
+if (!setDiagnostic && !reportDiags)
+  break;
+
+for (unsigned i = 0, e = opts.Remarks.size(); i != e; ++i) {
+  llvm::StringRef opt = opts.Remarks[i];
+  const auto flavor = clang::diag::Flavor::Remark;
+
+  // Check to see if this opt starts with "no-", if so, this is a
+  // neg

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy updated this revision to Diff 549881.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157104

Files:
  clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  clang/test/Analysis/out-of-bounds.c


Index: clang/test/Analysis/out-of-bounds.c
===
--- clang/test/Analysis/out-of-bounds.c
+++ clang/test/Analysis/out-of-bounds.c
@@ -151,11 +151,23 @@
 // Don't warn when indexing below the start of a symbolic region's whose
 // base extent we don't know.
 int *get_symbolic(void);
-void test_index_below_symboloc(void) {
+void test_underflow_symbolic(void) {
   int *buf = get_symbolic();
   buf[-1] = 0; // no-warning;
 }
 
+// But warn if we understand the internal memory layout of a symbolic region.
+typedef struct {
+  int id;
+  char name[256];
+} user_t;
+
+user_t *get_symbolic_user(void);
+char test_underflow_symbolic_2() {
+  user_t *user = get_symbolic_user();
+  return user->name[-1]; // expected-warning{{Out of bound memory access}}
+}
+
 void test_incomplete_struct(void) {
   extern struct incomplete incomplete;
   int *p = (int *)&incomplete;
Index: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -172,13 +172,18 @@
 return;
 
   NonLoc ByteOffset = RawOffset->getByteOffset();
+  const SubRegion *Reg = RawOffset->getRegion();
 
   // CHECK LOWER BOUND
-  const MemSpaceRegion *SR = RawOffset->getRegion()->getMemorySpace();
-  if (!llvm::isa(SR)) {
-// A pointer to UnknownSpaceRegion may point to the middle of
-// an allocated region.
-
+  const MemSpaceRegion *Space = Reg->getMemorySpace();
+  if (!(isa(Reg) && isa(Space))) {
+// A symbolic region in unknown space represents an unknown pointer that
+// may point into the middle of an array, so we don't look for underflows.
+// Both conditions are significant because we want to check underflows in
+// symbolic regions on the heap (which may be introduced by checkers like
+// MallocChecker that call SValBuilder::getConjuredHeapSymbolVal()) and
+// non-symbolic regions (e.g. a field subregion of a symbolic region) in
+// unknown space.
 auto [state_precedesLowerBound, state_withinLowerBound] =
 compareValueToThreshold(state, ByteOffset,
 svalBuilder.makeZeroArrayIndex(), svalBuilder);
@@ -195,7 +200,7 @@
 
   // CHECK UPPER BOUND
   DefinedOrUnknownSVal Size =
-  getDynamicExtent(state, RawOffset->getRegion(), svalBuilder);
+  getDynamicExtent(state, Reg, svalBuilder);
   if (auto KnownSize = Size.getAs()) {
 auto [state_withinUpperBound, state_exceedsUpperBound] =
 compareValueToThreshold(state, ByteOffset, *KnownSize, svalBuilder);


Index: clang/test/Analysis/out-of-bounds.c
===
--- clang/test/Analysis/out-of-bounds.c
+++ clang/test/Analysis/out-of-bounds.c
@@ -151,11 +151,23 @@
 // Don't warn when indexing below the start of a symbolic region's whose
 // base extent we don't know.
 int *get_symbolic(void);
-void test_index_below_symboloc(void) {
+void test_underflow_symbolic(void) {
   int *buf = get_symbolic();
   buf[-1] = 0; // no-warning;
 }
 
+// But warn if we understand the internal memory layout of a symbolic region.
+typedef struct {
+  int id;
+  char name[256];
+} user_t;
+
+user_t *get_symbolic_user(void);
+char test_underflow_symbolic_2() {
+  user_t *user = get_symbolic_user();
+  return user->name[-1]; // expected-warning{{Out of bound memory access}}
+}
+
 void test_incomplete_struct(void) {
   extern struct incomplete incomplete;
   int *p = (int *)&incomplete;
Index: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -172,13 +172,18 @@
 return;
 
   NonLoc ByteOffset = RawOffset->getByteOffset();
+  const SubRegion *Reg = RawOffset->getRegion();
 
   // CHECK LOWER BOUND
-  const MemSpaceRegion *SR = RawOffset->getRegion()->getMemorySpace();
-  if (!llvm::isa(SR)) {
-// A pointer to UnknownSpaceRegion may point to the middle of
-// an allocated region.
-
+  const MemSpaceRegion *Space = Reg->getMemorySpace();
+  if (!(isa(Reg) && isa(Space))) {
+// A symbolic region in unknown space represents an unknown pointer that
+// may point into the middle of an array, so we don't look for underflows.
+// Both conditions are significant because we want to check underflows in
+// symbolic regions on the heap (which may be introduced by checkers like
+// MallocChecker that call SValBuilder::get

[PATCH] D157813: [VE][Clang] Change to enable VPU flag by default

[Erich Focht via Phabricator via cfe-commits]

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

Looks good.
The default should be able to use intrinsics, hiding this only makes things 
complicated and gets users confused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157813

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

[PATCH] D157104: [analyzer] Improve underflow handling in ArrayBoundV2

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy marked 2 inline comments as done.
donat.nagy added a comment.

I didn't upload the test results yet because right now there is some 
incompatibility between our local fork and the upstream. I hope that it'll be 
fixed soon, but until then I can't use our automated tools to run the analysis 
with revisions that are close to the upstream main branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157104

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

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 549882.
victorkingi added a comment.

added tests for "no" variants of Rpass


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,52 @@
+! This file tests the -Rpass family of flags (-Rpass, -Rpass-missed
+! and -Rpass-analysis)
+! loop-delete isn't enabled at O0 so we use at least O1
+
+! Check that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check -Rno-pass, -Rno-pass-analysis, -Rno-pass-missed nothing emitted
+! RUN: %flang %s -O1 -Rno-pass 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-missed 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-analysis 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: optimization-remark.f90:49:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:44:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:44:5: remark: loop not vectorized: instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
\ No newline at end of file
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Option/Option.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/CommandLine.h"
+#include 
 
 namespace Fortran::frontend {
 
@@ -100,6 +101,61 @@
   llvm_unreachable("Invalid program action!");
 }
 
+// Emit a warning and typo hint for unknown warning opts
+static void emitUnknownDiagWarning(clang::DiagnosticsEngine &diags,
+   clang::diag::Flavor flavor,
+   llvm::StringRef prefix,
+   llvm::StringRef opt) {
+  llvm::StringRef suggestion =
+  clang::DiagnosticIDs::getNearestOption(flavor, opt);
+  diags.Report(clang::diag::warn_unknown_diag_option)
+  << (flavor == clang::diag::Flavor::WarningOrError ? 0 : 1)
+  << (prefix.str() += std::string(opt)) << !suggestion.empty()
+  << (prefix.str() += std::string(suggestion));
+}
+
+void processWarningOptions(clang::DiagnosticsEngine &diags,
+   const clang::DiagnosticOptions &opts,
+   bool reportDiags = true) {
+
+  llvm::SmallVector _diags;
+  const llvm::IntrusiveRefCntPtr diagIDs =
+  diags.getDiagnosticIDs();
+  // We parse the warning options twice.  The first pass sets diagnostic state,
+  // while the second pass reports warnings/errors.  This has the effect that
+  // we follow the more canonical "last option wins" paradigm when there are
+  // conflicting options.
+  for (unsigned report = 0, reportEnd = 2; report != reportEnd; ++report) {
+bool setDiagnostic = (report == 0);
+
+// If we've set the diagnostic state and are not reporting diagnostics then

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 549887.
victorkingi added a comment.

added comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,52 @@
+! This file tests the -Rpass family of flags (-Rpass, -Rpass-missed
+! and -Rpass-analysis)
+! loop-delete isn't enabled at O0 so we use at least O1
+
+! Check that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check -Rno-pass, -Rno-pass-analysis, -Rno-pass-missed nothing emitted
+! RUN: %flang %s -O1 -Rno-pass 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-missed 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-analysis 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: optimization-remark.f90:49:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:44:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:44:5: remark: loop not vectorized: instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
\ No newline at end of file
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Option/Option.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/CommandLine.h"
+#include 
 
 namespace Fortran::frontend {
 
@@ -100,6 +101,61 @@
   llvm_unreachable("Invalid program action!");
 }
 
+// Emit a warning and typo hint for unknown warning opts
+static void emitUnknownDiagWarning(clang::DiagnosticsEngine &diags,
+   clang::diag::Flavor flavor,
+   llvm::StringRef prefix,
+   llvm::StringRef opt) {
+  llvm::StringRef suggestion =
+  clang::DiagnosticIDs::getNearestOption(flavor, opt);
+  diags.Report(clang::diag::warn_unknown_diag_option)
+  << (flavor == clang::diag::Flavor::WarningOrError ? 0 : 1)
+  << (prefix.str() += std::string(opt)) << !suggestion.empty()
+  << (prefix.str() += std::string(suggestion));
+}
+
+void processWarningOptions(clang::DiagnosticsEngine &diags,
+   const clang::DiagnosticOptions &opts,
+   bool reportDiags = true) {
+
+  llvm::SmallVector _diags;
+  const llvm::IntrusiveRefCntPtr diagIDs =
+  diags.getDiagnosticIDs();
+  // We parse the warning options twice.  The first pass sets diagnostic state,
+  // while the second pass reports warnings/errors.  This has the effect that
+  // we follow the more canonical "last option wins" paradigm when there are
+  // conflicting options.
+  for (unsigned report = 0, reportEnd = 2; report != reportEnd; ++report) {
+bool setDiagnostic = (report == 0);
+
+// If we've set the diagnostic state and are not reporting diagnostics then
+// we're done.
+

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:156-158
+/// Parse a remark command line argument. It may be missing, disabled/enabled 
by
+/// '-R[no-]group' or specified with a regular expression by '-Rgroup=regexp'.
+/// On top of that, it can be disabled/enabled globally by '-R[no-]everything'.

awarzynski wrote:
> kiranchandramohan wrote:
> > awarzynski wrote:
> > > Could you give an example (and write a test ) for `-Rgroup=regexp`. Also, 
> > > @kiranchandramohan , is this form actually needed? I am thinking that 
> > > it's a complexity that could be avoided. It  could definitely simplify 
> > > this patch.
> > `Rpass=regex` is used, particularly `Rpass=.`. So this is required, but can 
> > be deferred to a separate patch to simplify this one.
> >  can be deferred to a separate patch to simplify this one
> 
> That would be a small win - the complexity comes from the fact that we can't 
> rely on TableGen to define all possible options. 
Removed need for Rpass=regex 



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:786
   parseShowColorsArgs(args, /*defaultDiagColor=*/false);
+  res.getDiagnosticOpts().ShowColors = res.getFrontendOpts().showColors;
 

awarzynski wrote:
> victorkingi wrote:
> > Apparently without forwarding the color option to the CompilerInvocation, 
> > flang doesn't print remark errors with color. Hence the need for this.
> > Also, a question, why do we have 2 instances of DiagnosticsEngine, one in 
> > CompilerInvocation and the other passed as an argument?
> > Apparently without forwarding the color option to the CompilerInvocation, 
> > flang doesn't print remark errors with color. Hence the need for this.
> 
> It is already "forwarded" here: 
> https://github.com/llvm/llvm-project/blob/3a100ea901ed79d6a06a5f018be2b4d3bbca51e8/flang/lib/Frontend/CompilerInvocation.cpp#L117-L122.
>  Could you explain why you need this change _here_?
> 
> > Also, a question, why do we have 2 instances of DiagnosticsEngine, one in 
> > CompilerInvocation and the other passed as an argument?
> 
> [[ 
> https://github.com/llvm/llvm-project/blob/3a100ea901ed79d6a06a5f018be2b4d3bbca51e8/flang/tools/flang-driver/fc1_main.cpp#L37-L40
>  | One  ]] is for the driver itself, to generate diagnostics related to 
> "driver errors" (e.g. option parsing errors). The [[ 
> https://github.com/llvm/llvm-project/blob/3a100ea901ed79d6a06a5f018be2b4d3bbca51e8/flang/include/flang/Frontend/CompilerInstance.h#L64-L65
>  | other ]] belongs to `CompilerInstance` rather than `CompilerInvocation` 
> and is used to report compilation errors (e.g. semantic or parsing errors). 
Thanks for the explanation, A bad regex error would be printed without color. 
But since we are getting rid of the regex option, might as well remove this.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:173-174
+if (!result.Regex->isValid(regexError)) {
+  diags.Report(clang::diag::err_drv_optimization_remark_pattern)
+  << regexError << a->getAsString(args);
+  return false;

awarzynski wrote:
> Could you add a test to exercise this diagnostic?
added the tests in `optimization-remark.f90` These are the `-Rno-pass`, 
`-Rno-pass-analysis` and `-Rno-pass-missed` tests.



Comment at: flang/lib/Frontend/FrontendActions.cpp:1007-1024
+  bool handleDiagnostics(const llvm::DiagnosticInfo &di) override {
+switch (di.getKind()) {
+case llvm::DK_OptimizationRemark:
+  optimizationRemarkHandler(llvm::cast(di));
+  break;
+case llvm::DK_OptimizationRemarkMissed:
+  
optimizationRemarkHandler(llvm::cast(di));

awarzynski wrote:
> Where is this method used?
`llvm/include/llvm/IR/DiagnosticHandler.h` specifies that this method needs to 
be overridden if one wants to setup custom diagnostic format reporting. 
`llvm/lib/IR/LLVMContext.cpp` then uses it for reporting in the diagnose 
function


```
void LLVMContext::diagnose(const DiagnosticInfo &DI) {
  if (auto *OptDiagBase = dyn_cast(&DI))
if (LLVMRemarkStreamer *RS = getLLVMRemarkStreamer())
  RS->emit(*OptDiagBase);

  // If there is a report handler, use it.
  if (pImpl->DiagHandler &&
  (!pImpl->RespectDiagnosticFilters || isDiagnosticEnabled(DI)) &&
  pImpl->DiagHandler->handleDiagnostics(DI))
return;
  .
  .
  .
```



Comment at: flang/lib/Frontend/TextDiagnosticPrinter.cpp:37
+// Print any diagnostic option information to a raw_ostream.
+static void printDiagnosticOptions(llvm::raw_ostream &os,
+   clang::DiagnosticsEngine::Level level,

awarzynski wrote:
> Why is this method needed and can it be tested?
This method prints out the pass that was done e.g. [-Rpass=inline ], 
[-Rpass=loop-delete] next to the optimization message.
It is tested by the full remark message emitted test in 
flang/test/

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi added inline comments.



Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:169-171
+  clang::ProcessWarningOptions(flang->getDiagnostics(),
+   flang->getDiagnosticOpts());
+

victorkingi wrote:
> awarzynski wrote:
> > Is this calling 
> > https://github.com/llvm/llvm-project/blob/c52d9509d40d3048914b144618232213e6076e05/clang/include/clang/Basic/Diagnostic.h#L1840-L1842?
> >  That's part of the `clangBasic` library. The overall goal in the driver is 
> > to reduce the dependency on Clang and this would be a step in the opposite 
> > direction. IIUC, we only need a small subset of that function, right?
> Yes, we only need a small subset. I'll create a static function in this file 
> to avoid the dependence
> Yes, we only need a small subset. I'll create a static function in this file 
> to avoid the dependence

I mean normal function not static


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156320

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

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi marked 6 inline comments as done.
victorkingi added inline comments.



Comment at: flang/lib/Frontend/FrontendActions.cpp:927
 
+class StandaloneBackendConsumer : public llvm::DiagnosticHandler {
+

awarzynski wrote:
> Why `StandaloneBackendConsumer`? Isn't this specifically for remarks? Also, 
> could you document this class a bit?
I've added a comment, does this give a good explanation of what it does?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156320

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

[PATCH] D157297: [clang] Fixes compile error like error: expected unqualified-id for ::_tzcnt_u32(mask);

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

In D157297#4583802 , @lygstate wrote:

> In D157297#4571572 , @pengfei wrote:
>
>> The description is not clear to me. You should describe the reason rather 
>> than phenomenon.
>>
>> My understanding is double colon operator cannot resolve functions with 
>> parentheses. https://godbolt.org/z/6P47se9WW
>
> error: expected unqualified-id
>
>   return ::_tzcnt_u32(a);
>^
>
> /opt/compiler-explorer/clang-16.0.0/lib/clang/16/include/bmiintrin.h:74:27: 
> note: expanded from macro '_tzcnt_u32'
> #define _tzcnt_u32(a) (__tzcnt_u32((a)))
>
> Looks like double colon operator cannot resolve macros with parentheses, is 
> that a compiler bug or we should handled it in code?
>
>> But I didn't find enough proof in Google. It'd be more persuasive if you can 
>> find it and add to the description.

GCC doesn't resolve it either. https://godbolt.org/z/Prb4s5d6o. So it should 
not be a bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157297

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

[PATCH] D152423: [RISCV] Add function that check extension name with version

[Piyou Chen via Phabricator via cfe-commits]

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

rebase and update testcase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152423

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/test/CodeGen/RISCV/riscv-func-attr-target.c

Index: clang/test/CodeGen/RISCV/riscv-func-attr-target.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/riscv-func-attr-target.c
@@ -0,0 +1,19 @@
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv64 -target-feature +zifencei \
+// RUN:  -target-feature +m -target-feature +a   \
+// RUN:  -target-feature +f -target-feature +d   \
+// RUN:  -emit-llvm %s -o - | FileCheck %s   \
+// RUN:  --check-prefix=CHECK-IR
+
+// CHECK-IR: void @test1() #0
+__attribute__((target("arch=-a,+v,+zbb,+zicond1p0"))) void test1() {}
+
+// CHECK-IR: void @test2() #1
+__attribute__((target("arch=rv64gc_zbb"))) void test2 () {}
+
+// CHECK-IR: void @test3() #2
+__attribute__((target("cpu=rocket-rv64;tune=generic-rv64;arch=+v"))) void test3 () {}
+
+// CHECK-IR: attributes #0 {{.*}}+experimental-zicond{{.*}}+v,+zbb{{.*}}+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b,-a{{.*}}
+// CHECK-IR: attributes #1 {{.*}}+c{{.*}}+zbb{{.*}}
+// CHECK-IR: attributes #2 {{.*}} "target-cpu"="rocket-rv64" {{.*}}+v{{.*}} "tune-cpu"="generic-rv64" {{.*}}
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -114,6 +114,9 @@
   void fillValidCPUList(SmallVectorImpl &Values) const override;
   bool isValidTuneCPUName(StringRef Name) const override;
   void fillValidTuneCPUList(SmallVectorImpl &Values) const override;
+  bool supportsTargetAttributeTune() const override { return true; }
+  bool validateCpuSupports(StringRef FeatureStr) const override;
+  ParsedTargetAttr parseTargetAttr(StringRef Str) const override;
 };
 class LLVM_LIBRARY_VISIBILITY RISCV32TargetInfo : public RISCVTargetInfo {
 public:
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -250,12 +250,17 @@
 
   // RISCVISAInfo makes implications for ISA features
   std::vector ImpliedFeatures = (*ParseResult)->toFeatureVector();
+  std::vector UpdatedFeatures;
+
   // Add non-ISA features like `relax` and `save-restore` back
   for (const std::string &Feature : FeaturesVec)
 if (!llvm::is_contained(ImpliedFeatures, Feature))
-  ImpliedFeatures.push_back(Feature);
+  UpdatedFeatures.push_back(Feature);
+
+  UpdatedFeatures.insert(UpdatedFeatures.end(), ImpliedFeatures.begin(),
+ ImpliedFeatures.end());
 
-  return TargetInfo::initFeatureMap(Features, Diags, CPU, ImpliedFeatures);
+  return TargetInfo::initFeatureMap(Features, Diags, CPU, UpdatedFeatures);
 }
 
 std::optional>
@@ -346,3 +351,77 @@
   bool Is64Bit = getTriple().isArch64Bit();
   llvm::RISCV::fillValidTuneCPUArchList(Values, Is64Bit);
 }
+
+// Parse RISC-V Target attributes, which are a comma separated list of:
+//  "arch=" - parsed to features as per -march=..
+//  "cpu=" - parsed to features as per -mcpu=.., with CPU set to 
+//  "tune=" - TuneCPU set to 
+ParsedTargetAttr RISCVTargetInfo::parseTargetAttr(StringRef Features) const {
+  ParsedTargetAttr Ret;
+  if (Features == "default")
+return Ret;
+  SmallVector AttrFeatures;
+  Features.split(AttrFeatures, ";");
+  bool FoundArch = false;
+
+  for (auto &Feature : AttrFeatures) {
+Feature = Feature.trim();
+StringRef AttrString = Feature.split("=").second.trim();
+
+if (Feature.startswith("arch=")) {
+  if (FoundArch)
+Ret.Duplicate = "arch=";
+  FoundArch = true;
+
+  if (AttrString.startswith("+") || AttrString.startswith("-")) {
+// EXTENSION like arch=+v,+zbb,-c
+SmallVector Exts;
+AttrString.split(Exts, ",");
+for (auto Ext : Exts) {
+  if (Ext.empty())
+continue;
+
+  StringRef ExtName = Ext.substr(1);
+  std::string TargetFeature =
+  llvm::RISCVISAInfo::getTargetFeatureForExtension(ExtName);
+  if (!TargetFeature.empty())
+Ret.Features.push_back(Ext.front() + TargetFeature);
+  else
+Ret.Features.push_back(Ext.str());
+}
+  } else {
+// full-arch-string like arch=rv64gcv
+auto RII = llvm::RISCVISAInfo::parseArchString(
+AttrString, /* EnableExperimentalExtension */ true);
+if (!RII) {
+  consumeError(RII.takeError());
+} else {
+  std::vector FeatStrings = (*RII)->toFeatureVe

[PATCH] D157837: [flang][driver] Update the visibility of Clang options in Flang

[David Truby via Phabricator via cfe-commits]

DavidTruby added a comment.

This looks good to me (minus the issue @tblah mentioned), while this will mean 
that we also error on gfortran options we don't recognize, I don't personally 
think that's a problem as in my personal experience Fortran users are used to 
different compilers accepting different options and dealing with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157837

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

[PATCH] D153701: [Clang] Implement P2718R0 "Lifetime extension in range-based for loops"

[Yurong via Phabricator via cfe-commits]

yronglin updated this revision to Diff 549896.
yronglin added a comment.

Add test to check generated LLVM IR, and fix crash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153701

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/AST/ast-dump-for-range-lifetime.cpp
  clang/test/CXX/special/class.temporary/p6.cpp

Index: clang/test/CXX/special/class.temporary/p6.cpp
===
--- clang/test/CXX/special/class.temporary/p6.cpp
+++ clang/test/CXX/special/class.temporary/p6.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++17 %s -triple x86_64-linux-gnu -emit-llvm -o - | FileCheck %s --implicit-check-not='call{{.*}}dtor'
+// RUN: %clang_cc1 -std=c++23 %s -triple x86_64-linux-gnu -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK-CXX23,CHECK-CXX23-NEXT
 
 namespace std {
   typedef decltype(sizeof(int)) size_t;
@@ -238,3 +239,236 @@
   // CHECK: call {{.*}}dtor
   // CHECK: }
 }
+
+namespace P2718R0 {
+
+// Test basic
+struct A {
+  int a[3] = {1, 2, 3};
+  A() {}
+  ~A() {}
+  const int *begin() const { return a; }
+  const int *end() const { return a + 3; }
+  A& r() { return *this; }
+  A g() { return A(); }
+};
+
+A g() { return A(); }
+const A &f1(const A &t) { return t; }
+
+void test1() {
+  [[maybe_unused]] int sum = 0;
+  // CHECK-CXX23: void @_ZN7P2718R05test1Ev()
+  // CHECK-CXX23: for.cond.cleanup:
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R01AD1Ev(
+  // CHECK-CXX23-NEXT: br label %for.end
+  for (auto e : f1(g()))
+sum += e;
+}
+
+struct B : A {};
+int (&f(const A *))[3];
+const A *g(const A &);
+void bar(int) {}
+
+void test2() {
+  // CHECK-CXX23: void @_ZN7P2718R05test2Ev()
+  // CHECK-CXX23: for.cond.cleanup:
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R01BD1Ev(
+  // CHECK-CXX23-NEXT: br label %for.end
+  for (auto e : f(g(B(
+bar(e);
+}
+
+// Test discard statement.
+struct LockGuard {
+LockGuard() {}
+~LockGuard() {}
+};
+
+void test3() {
+  int v[] = {42, 17, 13};
+
+  // CHECK-CXX23: void @_ZN7P2718R05test3Ev()
+  // CHECK-CXX23: for.cond.cleanup:
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R09LockGuardD1Ev(
+  // CHECK-CXX23-NEXT: br label %for.end
+  for ([[maybe_unused]] int x : static_cast(LockGuard()), v)
+LockGuard guard;
+  
+  // CHECK-CXX23: for.cond.cleanup11:
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R09LockGuardD1Ev(
+  // CHECK-CXX23-NEXT: br label %for.end17
+  for ([[maybe_unused]] int x : (void)LockGuard(), v)
+LockGuard guard;
+  
+  // CHECK-CXX23: for.cond.cleanup27:
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R09LockGuardD1Ev(
+  // CHECK-CXX23-NEXT: br label %for.end33
+  for ([[maybe_unused]] int x : LockGuard(), v)
+LockGuard guard;
+}
+
+// Test default arg
+int (&default_arg_fn(const A & = A()))[3];
+void test4() {
+
+  // CHECK-CXX23: void @_ZN7P2718R05test4Ev()
+  // CHECK-CXX23: for.cond.cleanup:
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R01AD1Ev(
+  // CHECK-CXX23-NEXT: br label %for.end
+  for (auto e : default_arg_fn()) 
+bar(e);
+}
+
+struct DefaultA {
+  DefaultA() {}
+  ~DefaultA() {}
+};
+
+A foo(const A&, const DefaultA &Default = DefaultA()) {
+  return A();
+}
+
+void test5() {
+  // CHECK-CXX23: void @_ZN7P2718R05test5Ev()
+  // CHECK-CXX23: for.cond.cleanup:
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R01AD1Ev(
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R08DefaultAD1Ev(
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R01AD1Ev(
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R08DefaultAD1Ev(
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R01AD1Ev(
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R08DefaultAD1Ev(
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R01AD1Ev(
+  // CHECK-CXX23-NEXT: br label %for.end
+  for (auto e : default_arg_fn(foo(foo(foo(A())
+bar(e);
+}
+
+struct C : public A {
+  C() {}
+  C(int, const C &, const DefaultA & = DefaultA()) {}
+};
+
+void test6() {
+  // CHECK-CXX23: void @_ZN7P2718R05test6Ev()
+  // CHECK-CXX23: for.cond.cleanup:
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R01CD1Ev(
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R08DefaultAD1Ev(
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R01CD1Ev(
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R08DefaultAD1Ev(
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R01CD1Ev(
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R08DefaultAD1Ev(
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R01CD1Ev(
+  // CHECK-CXX23: br label %for.end
+  for (auto e : C(0, C(0, C(0, C()
+bar(e);
+}
+
+// Test member call
+void test7() {
+  // CHECK-CXX23: void @_ZN7P2718R05test7Ev()
+  // CHECK-CXX23: for.cond.cleanup:
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R01AD1Ev(
+  // CHECK-CXX23-NEXT: call void @_ZN7P2718R01AD1Ev(
+  // CHECK-CXX23

[PATCH] D155610: [Clang][Sema] Fix display of characters on static assertion failure

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16869
+/// The code point needs to be zero-extended to 32-bits.
+void ConvertCharToString(uint32_t CodePoint, const BuiltinType *BTy,
+ unsigned TyWidth, llvm::raw_ostream &OS) {

hubert.reinterpretcast wrote:
> It does not seem that the first parameter expects a `CodePoint` argument in 
> all cases. For `Char_S`, `Char_U`, and `Char8`, it seems the function wants 
> to treat the input as a UTF-8 code unit.
> 
> I suggest changing the argument to be clearly a code unit (and potentially 
> treat it as a code point value as appropriate later in the function).
> 
> Also: The function should probably be declared as having static linkage.
> Additionally: The function does not "convert" in the language semantic sense. 
> `WriteCharacterValueDescriptionForDisplay` might be a better name.
Agreed, `CodeUnit` or `Value` would be more correct (mostly because of numeric 
escape sequences).
But if we are going to change that then `WriteCharValueForDiagnostic` would be 
better, `Character` implies too much



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16876
+  // other types.
+  if (CodePoint <= UCHAR_MAX) {
+StringRef Escaped = escapeCStyle(CodePoint);

hubert.reinterpretcast wrote:
> For types other than `Char_S`, `Char_U`, and `Char8`, this fails to treat the 
> C1 Controls and Latin-1 Supplement characters as Unicode code points. It 
> looks like test coverage for these cases are missing.
`escapeCStyle` is one of the things that assume ASCII / UTF, but yes, we might 
as well reduce to 0x7F just to avoid unnecessary work


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

https://reviews.llvm.org/D155610

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

[PATCH] D153701: [Clang] Implement P2718R0 "Lifetime extension in range-based for loops"

[Yurong via Phabricator via cfe-commits]

yronglin added a comment.

@cor3ntin I have reused `EnsureImmediateInvocationInDefaultArgs` to rewrite 
`CXXDefaultArgExpr`, does this is a correct approach?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153701

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

[PATCH] D86993: Document Clang's expectations of the C standard library.

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

@aaron.ballman I wanted to check back whether the linked document is what you 
had in mind, or whether some more/else would be needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86993

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

[PATCH] D157497: feat: Migrate isArch16Bit

[Evgeniy Makarev via Phabricator via cfe-commits]

Pivnoy added a comment.

This discussion was the main motivation for this change.
https://discourse.llvm.org/t/rfc-refactor-triple-related-classes/70410/11
As a result, Triple should become a data class, and its dependencies such as 
Architecture, Operating System etc. represent in the form of interfaces that 
can be implemented for the necessary instances.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157497

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

[PATCH] D157813: [VE][Clang] Change to enable VPU flag by default

[Kazushi Marukawa via Phabricator via cfe-commits]

kaz7 added a comment.

Thanks @efocht .  I appreciate if someone working on clang can review this 
patch too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157813

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

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

A simple test should be added to StructuralEquivalenceTest.cpp too to check if 
ignore (and not ignore) depth works.

I think this solution is not always correct, but is still an improvement.




Comment at: clang/include/clang/AST/ASTStructuralEquivalence.h:80
+  bool Complain = true, bool ErrorOnTagTypeMismatch = false,
+  bool IgnoreDepth = false)
   : FromCtx(FromCtx), ToCtx(ToCtx), NonEquivalentDecls(NonEquivalentDecls),

  bool IgnoreTemplateParmDepth = false)



Comment at: clang/lib/AST/ASTImporter.cpp:511
+bool IsStructuralMatch(Decl *From, Decl *To, bool Complain = true,
+   bool IgnoreDepth = true);
 ExpectedDecl VisitDecl(Decl *D);

This should be `false` to have the original behavior if not specified.



Comment at: clang/lib/AST/ASTImporter.cpp:5831
+FoundTemplate->getFriendObjectKind() != Decl::FOK_None &&
+!D->specializations().empty();
+if (IsStructuralMatch(D, FoundTemplate, true, IgnoreDepth)) {

Probably add `IsFriendTemplate`?



Comment at: clang/unittests/AST/ASTImporterTest.cpp:4258
+  auto *FromA = FirstDeclMatcher().match(
+  FromTU, classTemplateDecl(hasName("A")));
+  auto *ToA = Import(FromA, Lang_CXX11);

Probably add `hasDefinition()` to the matcher.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156693

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

[PATCH] D157814: [clang] Add missing field to TLSModelAttr json AST dump

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157814

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

[PATCH] D157795: [clang] Add missing field to SectionAttr json AST dump

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157795

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

[PATCH] D157080: [clangd] Symbol search includes parameter types for (member) functions

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

LSP is fairly clear here and our behavior here seems to be pretty correct.

- `name` is "the name of this symbol",
- detail is "More detail for this symbol, e.g the signature of a function.".

We're providing the type as detail: `void()` (for functions just `()` might be 
better, or better still: `() -> void`).

Anyway we're providing the data, VSCode is choosing not to display it. It does 
show it in the hierarchical symbol view (if you are inside the symbol and click 
on its name in the breadcrumbs along the top), and also in the outline view.
It seems that the VSCode folks have decided that the since the ctrl-shift-O 
view shows results as a flat list, the "grey text" slot is used to show 
hierarchy rather than detail.
I'm afraid that's up to them, you can file feature requests to have them change 
it.

Indeed it looks like the MS cpptools extension works around this by putting the 
detail into the name, but I don't think we should be emulating this: it 
violates the spec intent, is likely to break things in other editors. (cpptools 
don't care about this, they only support vscode).




Comment at: clang-tools-extra/clangd/Protocol.cpp:899
+  if (!S.detail.empty()) {
+if (S.kind == SymbolKind::Method || S.kind == SymbolKind::Function) {
+  llvm::StringRef Detail{S.detail};

DocumentSymbol is already a structure modelling LSP, this function's job is to 
serialize it, not rearrange the fields.

Moreover, we shouldn't render the detail into a string and then go back later 
and attempt to parse it, but just produce the data we want in the first place 
(in FindSymbols.cpp, where it can be unittested).


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

https://reviews.llvm.org/D157080

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

[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/JSONNodeDumper.cpp:541-548
+void JSONNodeDumper::VisitDeprecatedAttr(const DeprecatedAttr *DA) {
+  JOS.attribute("message", DA->getMessage());
+  JOS.attribute("replacement", DA->getReplacement());
+}
+
+void JSONNodeDumper::VisitUnavailableAttr(const UnavailableAttr *UA) {
+  JOS.attribute("message", UA->getMessage());

I think we should probably skip emitting the field if the field is empty. e.g., 
`[[deprecated("")]]` and `[[deprecated]]` should have the same output that 
doesn't have a `message` field; this keeps the JSON dumps somewhat more 
succinct for those cases. WDYT?


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

https://reviews.llvm.org/D157783

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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D112921#4581641 , @rnk wrote:

> What are the next steps here? Have concerns been addressed? The CI failures 
> seem unrelated (libcxx tests is an AIX bot out of disk, clang CI is an 
> interpreter test that seems unrelated, but please confirm).

We definitely need the libc++ reviewers to approve before we move forward. IMO. 
the clang bits look to be in reasonable shape in terms of code quality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D157775: [clang] Add aliasee field to AliasAttr json AST dump

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157775

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

[PATCH] D157781: [clang] Add cleanup_function field to CleanupAttr json AST dump

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157781

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

[PATCH] D157080: [clangd] Symbol search includes parameter types for (member) functions

[Florian Humblot via Phabricator via cfe-commits]

florianhumblot abandoned this revision.
florianhumblot added a comment.

@sammccall I guess you're right, I'll look into getting a fix into vscode 
instead.


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

https://reviews.llvm.org/D157080

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

[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added reviewers: sammccall, aaron.ballman.
Herald added a subscriber: arphaman.
Herald added a project: All.
kadircet requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

This makes sure we can preserve invalid-ness for consumers of this
node, it prevents crashes. It also aligns better with rest of the places that
store invalid expressions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157868

Files:
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/Index/complete-optional-params.cpp

Index: clang/test/Index/complete-optional-params.cpp
===
--- clang/test/Index/complete-optional-params.cpp
+++ clang/test/Index/complete-optional-params.cpp
@@ -79,7 +79,7 @@
 // CHECK-CC5-NEXT: Objective-C interface
 
 // RUN: c-index-test -code-completion-at=%s:17:11 %s | FileCheck -check-prefix=CHECK-CC6 %s
-// CHECK-CC6: FunctionDecl:{ResultType void}{TypedText foo_2}{LeftParen (}{Optional {Placeholder Bar1 b1 = Bar1()}{Optional {Comma , }{Placeholder Bar2 b2}}}{RightParen )} (50)
+// CHECK-CC6: FunctionDecl:{ResultType void}{TypedText foo_2}{LeftParen (}{Optional {Placeholder Bar1 b1 = Bar1()}{Optional {Comma , }{Placeholder Bar2 b2 = Bar2()}}}{RightParen )} (50)
 // CHECK-CC6: Completion contexts:
 // CHECK-CC6-NEXT: Any type
 // CHECK-CC6-NEXT: Any value
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -20,6 +20,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
@@ -37,11 +38,13 @@
 #include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
+#include "clang/Sema/Ownership.h"
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
@@ -329,23 +332,16 @@
   ParmVarDecl *Param = cast(param);
   UnparsedDefaultArgLocs.erase(Param);
 
-  auto Fail = [&] {
-Param->setInvalidDecl();
-Param->setDefaultArg(new (Context) OpaqueValueExpr(
-EqualLoc, Param->getType().getNonReferenceType(), VK_PRValue));
-  };
-
   // Default arguments are only permitted in C++
   if (!getLangOpts().CPlusPlus) {
 Diag(EqualLoc, diag::err_param_default_argument)
   << DefaultArg->getSourceRange();
-return Fail();
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
   }
 
   // Check for unexpanded parameter packs.
-  if (DiagnoseUnexpandedParameterPack(DefaultArg, UPPC_DefaultArgument)) {
-return Fail();
-  }
+  if (DiagnoseUnexpandedParameterPack(DefaultArg, UPPC_DefaultArgument))
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
 
   // C++11 [dcl.fct.default]p3
   //   A default argument expression [...] shall not be specified for a
@@ -360,14 +356,14 @@
 
   ExprResult Result = ConvertParamDefaultArgument(Param, DefaultArg, EqualLoc);
   if (Result.isInvalid())
-return Fail();
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
 
   DefaultArg = Result.getAs();
 
   // Check that the default argument is well-formed
   CheckDefaultArgumentVisitor DefaultArgChecker(*this, DefaultArg);
   if (DefaultArgChecker.Visit(DefaultArg))
-return Fail();
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
 
   SetParamDefaultArgument(Param, DefaultArg, EqualLoc);
 }
@@ -389,16 +385,24 @@
 
 /// ActOnParamDefaultArgumentError - Parsing or semantic analysis of
 /// the default argument for the parameter param failed.
-void Sema::ActOnParamDefaultArgumentError(Decl *param,
-  SourceLocation EqualLoc) {
+void Sema::ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc,
+  ExprResult DefaultArg) {
   if (!param)
 return;
 
   ParmVarDecl *Param = cast(param);
   Param->setInvalidDecl();
   UnparsedDefaultArgLocs.erase(Param);
-  Param->setDefaultArg(new (Context) OpaqueValueExpr(
-  EqualLoc, Param->getType().getNonReferenceType(), VK_PRValue));
+  ExprResult RE;
+  if (DefaultArg.isUsable()) {
+RE = CreateRecoveryExpr(EqualLoc, DefaultArg.get()->getEndLoc(),
+{DefaultArg.get()},
+   

[PATCH] D157869: [clang-tidy] Avoid overflow when dumping unsigned integer values

[Daniel Alcaide Nombela via Phabricator via cfe-commits]

ealcdan created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
ealcdan requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Some options take the maximum unsigned integer value as default, but
they are being dumped to a string as integers. This makes -dump-config
write invalid '-1' values for these options. This change fixes this
issue by using utostr if the option is unsigned.

Change-Id: I22d481047330a89984d6eb27bb02f85c71d42bdb

Fixes: #60217


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157869

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h


Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -350,12 +350,21 @@
 /// Stores an option with the check-local name \p LocalName with
 /// integer value \p Value to \p Options.
 template 
-std::enable_if_t::value>
+std::enable_if_t::value && std::is_signed::value>
 store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
   T Value) const {
   storeInt(Options, LocalName, Value);
 }
 
+/// Stores an option with the check-local name \p LocalName with
+/// unsigned integer value \p Value to \p Options.
+template 
+std::enable_if_t::value>
+store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+  T Value) const {
+  storeUnsigned(Options, LocalName, Value);
+}
+
 /// Stores an option with the check-local name \p LocalName as the string
 /// representation of the Enum \p Value to \p Options.
 ///
@@ -398,7 +407,8 @@
 
 void storeInt(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
   int64_t Value) const;
-
+void storeUnsigned(ClangTidyOptions::OptionMap &Options,
+   StringRef LocalName, uint64_t Value) const;
 
 std::string NamePrefix;
 const ClangTidyOptions::OptionMap &CheckOptions;
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -138,6 +138,12 @@
   store(Options, LocalName, llvm::itostr(Value));
 }
 
+void ClangTidyCheck::OptionsView::storeUnsigned(
+ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+uint64_t Value) const {
+  store(Options, LocalName, llvm::utostr(Value));
+}
+
 template <>
 void ClangTidyCheck::OptionsView::store(
 ClangTidyOptions::OptionMap &Options, StringRef LocalName,


Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -350,12 +350,21 @@
 /// Stores an option with the check-local name \p LocalName with
 /// integer value \p Value to \p Options.
 template 
-std::enable_if_t::value>
+std::enable_if_t::value && std::is_signed::value>
 store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
   T Value) const {
   storeInt(Options, LocalName, Value);
 }
 
+/// Stores an option with the check-local name \p LocalName with
+/// unsigned integer value \p Value to \p Options.
+template 
+std::enable_if_t::value>
+store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+  T Value) const {
+  storeUnsigned(Options, LocalName, Value);
+}
+
 /// Stores an option with the check-local name \p LocalName as the string
 /// representation of the Enum \p Value to \p Options.
 ///
@@ -398,7 +407,8 @@
 
 void storeInt(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
   int64_t Value) const;
-
+void storeUnsigned(ClangTidyOptions::OptionMap &Options,
+   StringRef LocalName, uint64_t Value) const;
 
 std::string NamePrefix;
 const ClangTidyOptions::OptionMap &CheckOptions;
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -138,6 +138,12 @@
   store(Options, LocalName, llvm::itostr(Value));
 }
 
+void ClangTidyCheck::OptionsView::storeUnsigned(
+ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+uint64_t Value) const {
+  store(Options, LocalName, llvm::utostr(Value));
+}
+
 template <>
 void ClangTidyCheck::OptionsView::store(
 ClangTidyOptions::OptionMap &Options, StringRef LocalName,
___
cfe-commits mailing list

[PATCH] D157663: [Driver] Default riscv*- triples to -fdebug-default-version=4

[Alex Bradbury via Phabricator via cfe-commits]

asb accepted this revision.
asb added a comment.

LGTM, this seems like a good workaround.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157663

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

[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

[Sam McCall via Phabricator via cfe-commits]

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

Nice, recoveryexpr is a better fit here. These changes tend to cause occasional 
new error-handling crashes on under-tested paths, but I guess it's a good time 
in the release cycle for that!

You might want a clangd or ast-dump test showing that we now preserve the 
internal structure of (some) invalid default initializes.




Comment at: clang/include/clang/Sema/Sema.h:3026
+  void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc,
+  ExprResult DefaultArg);
   ExprResult ConvertParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,

nit: Nullable `ExprResult*` since there are only two states?
Extra get() in some callers, but less mysterious



Comment at: clang/lib/Parse/ParseCXXInlineMethods.cpp:398
 DefArgResult = ParseAssignmentExpression();
-  DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult);
+  DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult, Param);
   if (DefArgResult.isInvalid()) {

If this fixes the recursive copy-constructor case you mentioned, can you add a 
test?

(Else does it do anything? Or just cleanup)



Comment at: clang/lib/Parse/ParseCXXInlineMethods.cpp:400
   if (DefArgResult.isInvalid()) {
-Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
+Actions.ActOnParamDefaultArgumentError(Param, EqualLoc, DefArgResult);
   } else {

DefArgResult is never anything here. I'd probably find `/*DefaultArg=*/nullptr` 
more obvious



Comment at: clang/lib/Parse/ParseDecl.cpp:7478
+Actions.ActOnParamDefaultArgumentError(Param, EqualLoc,
+   DefArgResult);
 SkipUntil(tok::comma, tok::r_paren, StopAtSemi | StopBeforeMatch);

Ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157868

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

[PATCH] D157454: [NFC][Clang] Fix static analyzer concern about null value dereference

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews added a comment.

Pre-merge check fails are unrelated - fatal error C1060: compiler is out of 
heap space


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

https://reviews.llvm.org/D157454

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

[clang] 421c9bb - [NFC][Clang] Fix static analyzer concern

[Elizabeth Andrews via cfe-commits]

Author: Elizabeth Andrews
Date: 2023-08-14T07:14:32-07:00
New Revision: 421c9bbf65b78e3410415cac2edf4e00bd4d38ca

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

LOG: [NFC][Clang] Fix static analyzer concern

Fix static analyzer concern about null value
dereference. InterfacePointerType is dereferenced
and should not be null.

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

Added: 


Modified: 
clang/lib/CodeGen/CGObjC.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGObjC.cpp b/clang/lib/CodeGen/CGObjC.cpp
index 46c37eaea82b1a..6c594b5db4bca1 100644
--- a/clang/lib/CodeGen/CGObjC.cpp
+++ b/clang/lib/CodeGen/CGObjC.cpp
@@ -222,6 +222,7 @@ llvm::Value 
*CodeGenFunction::EmitObjCCollectionLiteral(const Expr *E,
   QualType ResultType = E->getType();
   const ObjCObjectPointerType *InterfacePointerType
 = ResultType->getAsObjCInterfacePointerType();
+  assert(InterfacePointerType && "Unexpected InterfacePointerType - null");
   ObjCInterfaceDecl *Class
 = InterfacePointerType->getObjectType()->getInterface();
   CGObjCRuntime &Runtime = CGM.getObjCRuntime();



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

[PATCH] D157454: [NFC][Clang] Fix static analyzer concern about null value dereference

[Elizabeth Andrews 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 rG421c9bbf65b7: [NFC][Clang] Fix static analyzer concern 
(authored by eandrews).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157454

Files:
  clang/lib/CodeGen/CGObjC.cpp


Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -222,6 +222,7 @@
   QualType ResultType = E->getType();
   const ObjCObjectPointerType *InterfacePointerType
 = ResultType->getAsObjCInterfacePointerType();
+  assert(InterfacePointerType && "Unexpected InterfacePointerType - null");
   ObjCInterfaceDecl *Class
 = InterfacePointerType->getObjectType()->getInterface();
   CGObjCRuntime &Runtime = CGM.getObjCRuntime();


Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -222,6 +222,7 @@
   QualType ResultType = E->getType();
   const ObjCObjectPointerType *InterfacePointerType
 = ResultType->getAsObjCInterfacePointerType();
+  assert(InterfacePointerType && "Unexpected InterfacePointerType - null");
   ObjCInterfaceDecl *Class
 = InterfacePointerType->getObjectType()->getInterface();
   CGObjCRuntime &Runtime = CGM.getObjCRuntime();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153701: [Clang] Implement P2718R0 "Lifetime extension in range-based for loops"

[Yurong via Phabricator via cfe-commits]

yronglin added a comment.

In D153701#4563919 , @yronglin wrote:

> Sorry for the late reply.  I tried to investigate the memory impact of 
> creating these additional materializations by build the whole llvm-project 
> and compare the number of `MaterializedTemporaryExpr` created during parsing.
>
> Steps:
>
> 1. Add a public member `uint64_t NumMetarilizedTemporaryExpr = 0;` in 
> `ASTContext` .
> 2. Increment the value of `NumMetarilizedTemporaryExpr ` in 
> `Sema::CreateMaterializeTemporaryExpr`.
> 3. Write the `NumMetarilizedTemporaryExpr ` into a text file when 
> `ASTContext` destruction, each translation unit will append a line to the 
> file to record the value of `NumMetarilizedTemporaryExpr `.
> 4. Build the entire llvm-project separately using the compiler that creates 
> addational materializations and the compiler that doesn't.
> 5. Sum the numbers produced by each translation unit.
>
> The result is:
>
> | Item | Count |
> | Addational Materialized Temporarys Total | 50655585 |
> | Clang Trunk Total| 18346347 |
> | Diff | 32309238 |
> |
>
> The more detail result in https://yronglin.github.io
>
> The gap between these two numbers is very large. So I'think we can create 
> additional materializations only within for-range initializers. I'm not sure 
> if I can find a way to only create materializes for temporaries that need to 
> have an extended lifetime, WDYT?

I have updated the table, and it was sorted by `Count Inc` col.

**Num files: 7445**

| Item | Count | Mem  |
| Addational Materialized Temporarys Total | 50655585 | 1187240.2734 
(KB)  |
| Clang Trunk Total| 18346347 | 429992.5078 
(KB) |
| Diff | 32309238 | 757247.7656 
(KB) |
| Avg  | 4339.7230 | 4.2380 (KB/file) |


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153701

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

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro updated this revision to Diff 549925.
ro added a comment.
Herald added a subscriber: mgrang.

Updated based on discussions in Issue #53709:

- Sort Solaris GCC prefixes in reverse version order so the latest version is 
picked.
- Update testcase to match.

Tested on `amd64-pc-solaris2.11` and `x86_64-pc-linux-gnu`.

I'm seeing weird testsuite failures in 2-stage builds on Solaris/amd64 with gcc 
12, but not with clang 16, which I don't yet understand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157275

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0/sparcv8plus/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0/32/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/i386-pc-solaris2.11/4.7.3/amd64/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/i386-pc-solaris2.11/4.7.3/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/sparc-sun-solaris2.11/4.7.3/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/sparc-sun-solaris2.11/4.7.3/sparcv9/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/sparcv9-sun-solaris2.11/7.5.0/32/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/sparcv9-sun-solaris2.11/7.5.0/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/x86_64-pc-solaris2.11/7.5.0/32/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/x86_64-pc-solaris2.11/7.5.0/crtbegin.o
  clang/test/Driver/solaris-multi-gcc-search.test


Index: clang/test/Driver/solaris-multi-gcc-search.test
===
--- /dev/null
+++ clang/test/Driver/solaris-multi-gcc-search.test
@@ -0,0 +1,51 @@
+/// Check that clang uses the latest available Solaris GCC installation.
+
+/// Check sparc-sun-solaris2.11, 32-bit
+// RUN: %clang -v 2>&1 --target=sparc-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SPARC %s
+// CHECK-SPARC: Found candidate GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARC-NEXT: Selected GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARC-NEXT: Candidate multilib: .;@m64
+// CHECK-SPARC-NEXT: Candidate multilib: sparcv8plus;@m32
+// CHECK-SPARC-NEXT: Selected multilib: sparcv8plus;@m32
+
+/// Check i386-pc-solaris2.11, 32-bit
+// RUN: %clang -v 2>&1 --target=i386-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-I386 %s
+// CHECK-I386: Found candidate GCC installation: 
{{.*}}11/lib/gcc/x86_64-pc-solaris2.11/11.4.0
+// CHECK-I386-NEXT: Selected GCC installation: 
{{.*}}11/lib/gcc/x86_64-pc-solaris2.11/11.4.0
+// CHECK-I386-NEXT: Candidate multilib: .;@m64
+// CHECK-I386-NEXT: Candidate multilib: 32;@m32
+// CHECK-I386-NEXT: Selected multilib: 32;@m32
+
+/// Check sparcv9-sun-solaris2.11, 64-bit
+// RUN: %clang -v 2>&1 --target=sparcv9-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SPARCV9 %s
+// CHECK-SPARCV9: Found candidate GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARCV9-NEXT: Selected GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARCV9-NEXT: Candidate multilib: .;@m64
+// CHECK-SPARCV9-NEXT: Candidate multilib: sparcv8plus;@m32
+// CHECK-SPARCV9-NEXT: Selected multilib: .;@m64
+
+/// Check x86_64-pc-solaris2.11, 64-bit
+// RUN: %clang -v 2>&1 --target=x86_64-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-X86_64 %s
+
+/// Check amd64-pc-solaris2.11, 64-bit
+// RUN: %clang -v 2>&1 --target=amd64-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-X86_64 %s
+// CHECK-X86_64: Found candidate GCC installation: 
{{.*}}11/lib/gcc/x86_64-pc-solaris2.11/11.4.0
+// CHECK-X86_64-NEXT: Selected GCC installation: 
{{.*}}11/lib/gcc/x86_64-pc-solaris2.11/11.4.0
+// CHECK-X86_64-NEXT: Candidate multilib: .;@m64
+// CHECK-X86_64-NEXT: Candidate multilib: 32;@m32
+// CHECK-X86_64-NEXT: Selected multilib: .;@m64
Index: clang/lib/Driver/ToolChain

[PATCH] D155978: [SPIRV] Add SPIR-V logical triple.

[Nathan Gauër via Phabricator via cfe-commits]

Keenuts added inline comments.



Comment at: llvm/unittests/TargetParser/TripleTest.cpp:1307
+  EXPECT_TRUE(T.isSPIRV());
+
   T.setArch(Triple::spirv32);

Anastasia wrote:
> pmatos wrote:
> > Keenuts wrote:
> > > pmatos wrote:
> > > > I am not well-versed in SPIRV for gfx but if we are using 32bits in 
> > > > SPIRV logical, can't we reuse spirv32? Is there some sort of strong 
> > > > reason not to or adding spirv for logical spirv as an alternative to 
> > > > spirv32 and spirv64 is just easier?
> > > This is a very valid question! And I'm not sure of the best option.
> > > My understanding is: in logical SPIR-V, there are no notion of pointer 
> > > size, or architecture size. We cannot offset pointers by a sizeof(), or 
> > > do such things.
> > > 
> > > But the options I had didn't seem to allow this kind of "undefined 
> > > architecture".
> > > I chose 32bits here because the IDs are 32bit. But pointer addresses are 
> > > not, so returning 32bit is not quite right either.
> > > 
> > > 
> > This is a tricky one tbh - I would have to do a bit more research to have a 
> > more insighful reply here. Maybe @mpaszkowski or @Anastasia can add more.
> I think to do this properly in LLVM would require an IR extension or 
> something, it is maybe worth starting RFC to discuss this.
> 
> Out of curiosity do you have any code that can be compiled from any GFX 
> source language that would need a pointer size to be emitted in IR? If there 
> is no code that can be written in such a way then using one fixed pointer 
> width could be ok. Then the SPIR-V backend should just recognise it and lower 
> as required. Otherwise target configurable types might be useful: 
> https://llvm.org/docs/LangRef.html#target-extension-type
> 
> In general we tried to avoid adding bitwidth neutral SPIR-V triple originally 
> just because LLVM always requires a pointer width. We were thinking of adding 
> vulkan as a component to the existing SPIR-V 34|64 targets 
> https://discourse.llvm.org/t/setting-version-environment-and-extensions-for-spir-v-target.
Hello!

Thanks for the feedback!
I asked around, and it seems that no, we cannot write code that pointer 
arithmetic or requires the pointer size that I know of.
The code that could require is changes the memory modal to something else, like 
this proposal: 
https://github.com/microsoft/hlsl-specs/blob/main/proposals/0010-vk-buffer-ref.md
But here, the memory model is PhysicalStorageBuffer64, which tells us pointers 
are 64bit.

We also have the SPV_KHR_variable_pointers extension 
(https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/KHR/SPV_KHR_variable_pointers.asciidoc)
which specifically says pointers have no size or bit pattern...


> Otherwise target configurable types might be useful

Not familiar enough with LLVM/Clang to fully understand, so reformulating we'd 
have:
- the target would return 32-bit, even if we have no pointer size
- the IR would not use classic pointers at all, since their type would be 
misleading.
- the IR would use this newly created type (let's call is LogicalPointer) 
instead of real pointers.
- the backend would then convert this new type to something else when lowering.

If that's the case, seems fair, I'd follow this advice.

> We were thinking of adding vulkan as a component to the existing SPIR-V 34|64 
> targets
Same idea here, add VulkanX to the OSType part of the triple (replacing the 
ShaderModel used for DXIL).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155978

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

[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 549929.
serge-sans-paille added a comment.

Only output extra fields if they are not empty


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

https://reviews.llvm.org/D157783

Files:
  clang/include/clang/AST/JSONNodeDumper.h
  clang/lib/AST/JSONNodeDumper.cpp
  clang/test/AST/ast-dump-attr-json.cpp

Index: clang/test/AST/ast-dump-attr-json.cpp
===
--- clang/test/AST/ast-dump-attr-json.cpp
+++ clang/test/AST/ast-dump-attr-json.cpp
@@ -8,6 +8,13 @@
   __attribute__((cleanup(cleanup_function))) int var;
 }
 
+__attribute__((deprecated)) int deprecated_var0;
+__attribute__((deprecated("reason"))) int deprecated_var1;
+__attribute__((deprecated("reason", "replacement"))) int deprecated_var2;
+
+__attribute__((unavailable)) int unavailable_var0;
+__attribute__((unavailable("reason"))) int unavailable_var1;
+
 // NOTE: CHECK lines have been autogenerated by gen_ast_dump_json_test.py
 // using --filters=VarDecl
 
@@ -139,3 +146,237 @@
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 282,
+// CHECK-NEXT:   "line": 11,
+// CHECK-NEXT:   "col": 33,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 250,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 282,
+// CHECK-NEXT:"col": 33,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var0",
+// CHECK-NEXT:  "mangledName": "deprecated_var0",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 265,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 265,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: }
+// CHECK-NEXT:}
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ]
+// CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 341,
+// CHECK-NEXT:   "line": 12,
+// CHECK-NEXT:   "col": 43,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 299,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 341,
+// CHECK-NEXT:"col": 43,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var1",
+// CHECK-NEXT:  "mangledName": "deprecated_var1",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 314,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 333,
+// CHECK-NEXT:  "col": 35,
+// CHECK-NEXT:  "tokLen": 1
+// CHECK-NEXT: }
+// CHECK-NEXT:},
+// CHECK-NEXT:"message": "reason"
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ]
+// CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 415,
+// CHECK-NEXT:   "line": 13,
+// CHECK-NEXT:   "col": 58,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 358,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 415,
+// CHECK-NEXT:"col": 58,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var2",
+// CHECK-NEXT:  "mangledName": "deprecated_var2",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 373,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 407,
+// CHECK-NEXT:  "col": 50,
+// CHECK-NEXT:  "tokLen": 1
+// CHECK-NEXT: }
+// CHECK-NEXT:},
+// CHECK

[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

[serge via Phabricator via cfe-commits]

serge-sans-paille marked an inline comment as done.
serge-sans-paille added a comment.

Review taken into account. Thanks for handling this batch of patches o/


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

https://reviews.llvm.org/D157783

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

[PATCH] D157879: [clang] Report missing designated initializers in C++

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Herald added a project: All.
Fznamznon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Prior to this change clang didn't emit missing-field-initializers
warning for designated initializers. The comments say that it is done to
match gcc behavior. However, gcc behaves so only for C. For C++ warnings
are emitted.

Fixes https://github.com/llvm/llvm-project/issues/56628


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157879

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp

Index: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
===
--- clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
+++ clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,reorder -Wno-c99-designator -Werror=reorder-init-list -Wno-initializer-overrides
 // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,override -Wno-c99-designator -Wno-reorder-init-list -Werror=initializer-overrides
 // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected -Wno-c99-designator -Wno-reorder-init-list -Wno-initializer-overrides
+// RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,wmissing -Wmissing-field-initializers -Wno-c99-designator -Wno-reorder-init-list -Wno-initializer-overrides
 
 
 namespace class_with_ctor {
@@ -49,15 +50,18 @@
 A a4 = {
   .x = 1, // override-note {{previous}}
   .x = 1 // override-error {{overrides prior initialization}}
-};
+}; // wmissing-warning {{missing field 'y' initializer}}
 A a5 = {
   .y = 1, // override-note {{previous}}
   .y = 1 // override-error {{overrides prior initialization}}
-};
+}; // wmissing-warning {{missing field 'x' initializer}}
 B b2 = {.a = 1}; // pedantic-error {{brace elision for designated initializer is a C99 extension}}
+ // wmissing-warning@-1 {{missing field 'y' initializer}}
 B b3 = {.a = 1, 2}; // pedantic-error {{mixture of designated and non-designated}} pedantic-note {{first non-designated}} pedantic-error {{brace elision}}
 B b4 = {.a = 1, 2, 3}; // pedantic-error {{mixture of designated and non-designated}} pedantic-note {{first non-designated}} pedantic-error {{brace elision}} expected-error {{excess elements}}
 B b5 = {.a = nullptr}; // expected-error {{cannot initialize}}
+   // wmissing-warning@-1 {{missing field 'y' initializer}}
+   // wmissing-warning@-2 {{missing field 'a' initializer}}
 struct C { int :0, x, :0, y, :0; };
 C c = {
   .x = 1, // override-note {{previous}}
@@ -66,7 +70,14 @@
   .y = 1, // override-error {{overrides prior initialization}} // reorder-note {{previous initialization for field 'y' is here}}
   .x = 1, // reorder-error {{declaration order}} override-error {{overrides prior initialization}} override-note {{previous}}
   .x = 1, // override-error {{overrides prior initialization}}
-};
+}; //wmissing-warning {{missing field 'x' initializer}}
+
+struct Foo { int a, b; };
+
+struct Foo foo0 = { 1 }; // wmissing-warning {{missing field 'b' initializer}}
+struct Foo foo1 = { .a = 1 }; // wmissing-warning {{missing field 'b' initializer}}
+struct Foo foo2 = { .b = 1 }; // wmissing-warning {{missing field 'a' initializer}}
+
 }
 
 namespace base_class {
@@ -215,5 +226,5 @@
 .c = 1, // reorder-error {{field 'd' will be initialized after field 'c'}} // reorder-note {{previous initialization for field 'c' is here}}
 .b = 1, // reorder-error {{field 'c' will be initialized after field 'b'}} // reorder-note {{previous initialization for field 'b' is here}}
 .a = 1, // reorder-error {{field 'b' will be initialized after field 'a'}}
-};
+}; // wmissing-warning {{missing field 'a' initializer}}
 }
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -2252,6 +2252,8 @@
 !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts());
   bool HasDesignatedInit = false;
 
+  llvm::SmallPtrSet InitializedFields;
+
   while (Index < IList->getNumInits()) {
 Expr *Init = IList->getInit(Index);
 SourceLocation InitLoc = Init->getBeginLoc();
@@ -2277,6 +2279,7 @@
 RecordDecl::field_iterator F = RD->field_begin();
 while (std::next(F) != Field)
   ++F;
+InitializedFields.insert(*F);
 QualType ET = SemaRef.Context.getBaseElementType(F->getType());
 if (checkDestructorReference(ET, InitLoc, SemaRef)) {
   hadError = true;
@@ -2288,7 +2291,8 @@
 
   // Disable check for missing fields when designators are used.
   // This matches gcc behaviour.
-  CheckForMissingFields = false;
+  if (!SemaRef.getLangOpts().CPlusPlus)
+CheckForMissingFields = false;
   continue;
 }
 
@@ -2367,6 +2371,7 @@
 CheckSubElementType(M

[PATCH] D157869: [clang-tidy] Avoid overflow when dumping unsigned integer values

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

This wont fix issue, simply because problem happen during reading -1, not 
during dumping, and per documentation -1 was a valid value. Same issue happen 
in other checks that use -1. Most probably best way would be to change those 
variables into std::int64_t, in C++ creating any structure of size above max 
int will fail with error anyway, so using signed i64 should be sufficient, and 
would also accept -1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157869

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

[PATCH] D157869: [clang-tidy] Avoid overflow when dumping unsigned integer values

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

I agree that this change is an improvement that is welcome, but that's not a 
fix for a #60217.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157869

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

[PATCH] D142660: [AIX] supporting -X options for llvm-ranlib in AIX OS

[Digger Lin via Phabricator via cfe-commits]

DiggerLin updated this revision to Diff 549938.
DiggerLin marked 4 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142660

Files:
  clang/lib/Driver/OffloadBundler.cpp
  clang/test/lit.cfg.py
  clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
  llvm/include/llvm/Object/Archive.h
  llvm/include/llvm/Object/ArchiveWriter.h
  llvm/lib/ObjCopy/Archive.cpp
  llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp
  llvm/lib/Object/Archive.cpp
  llvm/lib/Object/ArchiveWriter.cpp
  llvm/lib/Object/COFFImportFile.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/test/tools/llvm-ranlib/AIX-X-option-non-AIX.test
  llvm/test/tools/llvm-ranlib/aix-X-option.test
  llvm/tools/llvm-ar/llvm-ar.cpp
  llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp

Index: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
===
--- llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
+++ llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
@@ -594,18 +594,17 @@
 
   if (NewMembers.size() == 1)
 return writeArchive(OutputFile, NewMembers.begin()->second.getMembers(),
-/*WriteSymtab=*/true,
+SymtabWritingMode::NormalSymtab,
 /*Kind=*/object::Archive::K_DARWIN, C.Deterministic,
 /*Thin=*/false);
 
   SmallVector, 2> OutputBinaries;
   for (const std::pair &M : NewMembers) {
 Expected> OutputBufferOrErr =
-writeArchiveToBuffer(M.second.getMembers(),
- /*WriteSymtab=*/true,
- /*Kind=*/object::Archive::K_DARWIN,
- C.Deterministic,
- /*Thin=*/false);
+writeArchiveToBuffer(
+M.second.getMembers(), SymtabWritingMode::NormalSymtab,
+/*Kind=*/object::Archive::K_DARWIN, C.Deterministic,
+/*Thin=*/false);
 if (!OutputBufferOrErr)
   return OutputBufferOrErr.takeError();
 std::unique_ptr &OutputBuffer = OutputBufferOrErr.get();
Index: llvm/tools/llvm-ar/llvm-ar.cpp
===
--- llvm/tools/llvm-ar/llvm-ar.cpp
+++ llvm/tools/llvm-ar/llvm-ar.cpp
@@ -69,7 +69,9 @@
  << "  -v --version  - Display the version of this program\n"
  << "  -D- Use zero for timestamps and uids/gids "
 "(default)\n"
- << "  -U- Use actual timestamps and uids/gids\n";
+ << "  -U- Use actual timestamps and uids/gids\n"
+ << "  -X{32|64|32_64|any}   - Specify which archive symbol tables "
+"should be generated if they do not already exist (AIX OS only)\n";
 }
 
 static void printArHelp(StringRef ToolName) {
@@ -225,7 +227,8 @@
 static bool CompareFullPath = false;  ///< 'P' modifier
 static bool OnlyUpdate = false;   ///< 'u' modifier
 static bool Verbose = false;  ///< 'v' modifier
-static bool Symtab = true;///< 's' modifier
+static SymtabWritingMode Symtab =
+SymtabWritingMode::NormalSymtab;  ///< 's' modifier
 static bool Deterministic = true; ///< 'D' and 'U' modifiers
 static bool Thin = false; ///< 'T' modifier
 static bool AddLibrary = false;   ///< 'L' modifier
@@ -371,11 +374,11 @@
   CompareFullPath = true;
   break;
 case 's':
-  Symtab = true;
+  Symtab = SymtabWritingMode::NormalSymtab;
   MaybeJustCreateSymTab = true;
   break;
 case 'S':
-  Symtab = false;
+  Symtab = SymtabWritingMode::NoSymtab;
   break;
 case 'u':
   OnlyUpdate = true;
@@ -1074,9 +1077,31 @@
   // In summary, we only need to update the symbol table if we have none.
   // This is actually very common because of broken build systems that think
   // they have to run ranlib.
-  if (OldArchive->hasSymbolTable())
-return;
+  if (OldArchive->hasSymbolTable()) {
+if (OldArchive->kind() != object::Archive::K_AIXBIG)
+  return;
 
+// For archives in the Big Archive format, the bit mode option specifies
+// which symbol table to generate. The presence of a symbol table that does
+// not match the specified bit mode does not prevent creation of the symbol
+// table that has been requested.
+if (OldArchive->kind() == object::Archive::K_AIXBIG) {
+  BigArchive *BigArc = dyn_cast(OldArchive);
+  if (BigArc->has32BitGlobalSymtab() &&
+  Symtab == SymtabWritingMode::BigArchive32)
+return;
+
+  if (BigArc->has64BitGlobalSymtab() &&
+  Symtab == SymtabWritingMode::BigArchive64)
+return;
+
+  if (BigArc->has32BitGlobalSymtab() && BigArc->has64BitGlobalSymtab() &&
+  Symtab == SymtabWritingMode::NormalSymtab)
+return;
+
+  Symtab = SymtabWritin

[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Use Expr* instead of ExprResult
- Add dump test to demonstrate new RecoveryExpr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157868

Files:
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/AST/ast-dump-default-arg-recovery.cpp
  clang/test/Index/complete-optional-params.cpp

Index: clang/test/Index/complete-optional-params.cpp
===
--- clang/test/Index/complete-optional-params.cpp
+++ clang/test/Index/complete-optional-params.cpp
@@ -79,7 +79,7 @@
 // CHECK-CC5-NEXT: Objective-C interface
 
 // RUN: c-index-test -code-completion-at=%s:17:11 %s | FileCheck -check-prefix=CHECK-CC6 %s
-// CHECK-CC6: FunctionDecl:{ResultType void}{TypedText foo_2}{LeftParen (}{Optional {Placeholder Bar1 b1 = Bar1()}{Optional {Comma , }{Placeholder Bar2 b2}}}{RightParen )} (50)
+// CHECK-CC6: FunctionDecl:{ResultType void}{TypedText foo_2}{LeftParen (}{Optional {Placeholder Bar1 b1 = Bar1()}{Optional {Comma , }{Placeholder Bar2 b2 = Bar2()}}}{RightParen )} (50)
 // CHECK-CC6: Completion contexts:
 // CHECK-CC6-NEXT: Any type
 // CHECK-CC6-NEXT: Any value
Index: clang/test/AST/ast-dump-default-arg-recovery.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-default-arg-recovery.cpp
@@ -0,0 +1,6 @@
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump -frecovery-ast %s | FileCheck %s
+
+void foo();
+void fun(int arg = foo());
+//  CHECK: -ParmVarDecl {{.*}}  col:14 invalid arg 'int' cinit
+// CHECK-NEXT:   -RecoveryExpr {{.*}}  'int' contains-errors
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -20,6 +20,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
@@ -37,11 +38,13 @@
 #include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
+#include "clang/Sema/Ownership.h"
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
@@ -329,23 +332,16 @@
   ParmVarDecl *Param = cast(param);
   UnparsedDefaultArgLocs.erase(Param);
 
-  auto Fail = [&] {
-Param->setInvalidDecl();
-Param->setDefaultArg(new (Context) OpaqueValueExpr(
-EqualLoc, Param->getType().getNonReferenceType(), VK_PRValue));
-  };
-
   // Default arguments are only permitted in C++
   if (!getLangOpts().CPlusPlus) {
 Diag(EqualLoc, diag::err_param_default_argument)
   << DefaultArg->getSourceRange();
-return Fail();
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
   }
 
   // Check for unexpanded parameter packs.
-  if (DiagnoseUnexpandedParameterPack(DefaultArg, UPPC_DefaultArgument)) {
-return Fail();
-  }
+  if (DiagnoseUnexpandedParameterPack(DefaultArg, UPPC_DefaultArgument))
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
 
   // C++11 [dcl.fct.default]p3
   //   A default argument expression [...] shall not be specified for a
@@ -360,14 +356,14 @@
 
   ExprResult Result = ConvertParamDefaultArgument(Param, DefaultArg, EqualLoc);
   if (Result.isInvalid())
-return Fail();
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
 
   DefaultArg = Result.getAs();
 
   // Check that the default argument is well-formed
   CheckDefaultArgumentVisitor DefaultArgChecker(*this, DefaultArg);
   if (DefaultArgChecker.Visit(DefaultArg))
-return Fail();
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
 
   SetParamDefaultArgument(Param, DefaultArg, EqualLoc);
 }
@@ -389,16 +385,23 @@
 
 /// ActOnParamDefaultArgumentError - Parsing or semantic analysis of
 /// the default argument for the parameter param failed.
-void Sema::ActOnParamDefaultArgumentError(Decl *param,
-  SourceLocation EqualLoc) {
+void Sema::ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc,
+  Expr *DefaultArg) {
   if (!param)
 return;
 
   ParmVarDecl *Param = cast(param);
   Param->setInvalidDecl();
   UnparsedDefa

[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:3026
+  void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc,
+  ExprResult DefaultArg);
   ExprResult ConvertParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,

sammccall wrote:
> nit: Nullable `ExprResult*` since there are only two states?
> Extra get() in some callers, but less mysterious
i guess you meant `Expr*` ?



Comment at: clang/lib/Parse/ParseCXXInlineMethods.cpp:398
 DefArgResult = ParseAssignmentExpression();
-  DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult);
+  DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult, Param);
   if (DefArgResult.isInvalid()) {

sammccall wrote:
> If this fixes the recursive copy-constructor case you mentioned, can you add 
> a test?
> 
> (Else does it do anything? Or just cleanup)
no it doesn't. unfortunately that requires change in a separate code path to 
mark any method decls with invalid parmvardecls as invalid, which i'll try to 
put together. as that's the behavior for regular functiondecls, i don't see why 
it should be different for methods (if so we should probably change the 
behavior for functions instead).



Comment at: clang/lib/Parse/ParseCXXInlineMethods.cpp:400
   if (DefArgResult.isInvalid()) {
-Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
+Actions.ActOnParamDefaultArgumentError(Param, EqualLoc, DefArgResult);
   } else {

sammccall wrote:
> DefArgResult is never anything here. I'd probably find 
> `/*DefaultArg=*/nullptr` more obvious
maybe i am missing something, but "invalid" doesn't mean "unusable".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157868

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

[PATCH] D142660: [AIX] supporting -X options for llvm-ranlib in AIX OS

[Digger Lin via Phabricator via cfe-commits]

DiggerLin added inline comments.



Comment at: clang/test/lit.cfg.py:391
 if "system-aix" in config.available_features:
-config.substitutions.append(("llvm-nm", "env OBJECT_MODE=any llvm-nm"))
-config.substitutions.append(("llvm-ar", "env OBJECT_MODE=any llvm-ar"))
+   config.environment["OBJECT_MODE"] ="any"
 

jhenderson wrote:
> It might be a good idea for you to run the `black` tool on python changes, to 
> ensure they conform to the coding standard, much like you do with 
> clang-format for C++ changes.
thanks for let me know.



Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:74
+ << "  -X{32|64|32_64|any}   - Specify which archive symbol tables "
+"should be generated if they do not already exist (AIX OS only)\n";
 }

jhenderson wrote:
> Strictly speaking, this should be "Big Archive formats only" not "AIX OS 
> only" since theoretically you could have Big Archive archives on other 
> platforms. However, I'm not bothered if you don't want to change this. The 
> same probably goes for other tools for that matter, but don't change them now.
Strictly speaking, this should be "Big Archive format on AIX OS only" ,
in AIX OS , you can still input the -X option , but the X option not work for 
gnu archive format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142660

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

[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


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

https://reviews.llvm.org/D157783

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

[PATCH] D157885: [NFC][Clang] Fix static analyzer concern about null value derefence

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews created this revision.
eandrews added reviewers: aaron.ballman, tahonermann.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, 
a.sidorin, baloghadamsoftware.
Herald added a project: All.
eandrews requested review of this revision.

https://reviews.llvm.org/D157885

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp


Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4459,7 +4459,9 @@
   if (auto *VTSD = dyn_cast(D)) {
 VTSD->setPointOfInstantiation(POI);
   } else if (auto *VD = dyn_cast(D)) {
-VD->getMemberSpecializationInfo()->setPointOfInstantiation(POI);
+MemberSpecializationInfo *MSInfo = VD->getMemberSpecializationInfo();
+assert(MSInfo && "No member specialization information");
+MSInfo->setPointOfInstantiation(POI);
   } else {
 auto *FD = cast(D);
 if (auto *FTSInfo = FD->TemplateOrSpecialization


Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4459,7 +4459,9 @@
   if (auto *VTSD = dyn_cast(D)) {
 VTSD->setPointOfInstantiation(POI);
   } else if (auto *VD = dyn_cast(D)) {
-VD->getMemberSpecializationInfo()->setPointOfInstantiation(POI);
+MemberSpecializationInfo *MSInfo = VD->getMemberSpecializationInfo();
+assert(MSInfo && "No member specialization information");
+MSInfo->setPointOfInstantiation(POI);
   } else {
 auto *FD = cast(D);
 if (auto *FTSInfo = FD->TemplateOrSpecialization
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Thanks for the update. Regarding testing, it seems that unittests will be more 
convenience than creating so many placeholder files in `Inputs/`. 
`clang/unittests/Driver/ToolChainTest.cpp` `TEST(ToolChainTest, 
VFSGCCInstallation)` has an example for Linux. You can add another for Solaris.




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2258
 
-  Prefixes.push_back(CandidatePrefix);
+  SolarisPrefixes.push_back(CandidatePrefix);
 }

It's better to use https://en.wikipedia.org/wiki/Schwartzian_transform here to 
avoid calling `llvm::sys::path::filename` and `Generic_GCC::GCCVersion::Parse` 
in the comparator. Then the comparator is just `A < B` and can just be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157275

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

[PATCH] D157808: [clang] Add missing field to VisibilityAttr json AST dump

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


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

https://reviews.llvm.org/D157808

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

[clang] bbc0f99 - [Driver] Default riscv*- triples to -fdebug-default-version=4

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2023-08-14T08:26:52-07:00
New Revision: bbc0f99f3bc96f1db16f649fc21dd18e5b0918f6

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

LOG: [Driver] Default riscv*- triples to -fdebug-default-version=4

This adds a RISC-V special case to ToolChain::GetDefaultDwarfVersion,
affecting Linux/Haiku/RISCVToolChain.

DWARF v5 .debug_loclists/.debug_rnglists's
DW_LLE_offset_pair/DW_RLE_offset_pair entry kinds utilitize `.uleb128 A-B`
directives where A and B reference local labels in code sections.
When A and B are separated by a RISC-V linker-relaxable instruction,
A-B is incorrectly folded without a relocation, causing incorrect debug
information.

```
void ext(void);
int foo(int x) {ext(); return 0;}
// DW_AT_location [DW_FORM_loclistx] of a DW_TAG_formal_parameter references a 
DW_LLE_offset_pair that can be incorrect after linker relaxation.

int ext(void);
void foo() { {
  int ret = ext();
  if (__builtin_expect(ret, 0))
ext();
} }
// DW_AT_ranges [DW_FORM_rnglistx] of a DW_TAG_lexical_block references a 
DW_RLE_offset_pair that can be incorrect after linker relaxation.
```

D157657 will implement R_RISCV_SET_ULEB128/R_RISCV_SUB_ULEB128
relocations, fixing the issue, but the relocation is only supported by
bleeding-edge binutils 2.41 and not by lld/ELF yet.

The goal is to make the emitted DWARF correct after linking.
Many users don't care about the default DWARF version, but a linker
error will be unacceptable. Let's just downgrade the default DWARF
version, before binutils>=2.41 is more widely available.

An alternative compatibility option is to add a toggle to DwarfDebug.cpp,
but that doesn't seem like a good idea.

Reviewed By: asb, kito-cheng

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

Added: 


Modified: 
clang/include/clang/Driver/ToolChain.h
clang/lib/Driver/ToolChain.cpp
clang/test/Driver/clang-g-opts.c

Removed: 




diff  --git a/clang/include/clang/Driver/ToolChain.h 
b/clang/include/clang/Driver/ToolChain.h
index e3fcbd9322b0e4..2e74507f71267c 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -561,7 +561,7 @@ class ToolChain {
 
   // Return the DWARF version to emit, in the absence of arguments
   // to the contrary.
-  virtual unsigned GetDefaultDwarfVersion() const { return 5; }
+  virtual unsigned GetDefaultDwarfVersion() const;
 
   // Some toolchains may have 
diff erent restrictions on the DWARF version and
   // may need to adjust it. E.g. NVPTX may need to enforce DWARF2 even when 
host

diff  --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index d60fdbc179683b..8dafc3d481c2e0 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -427,6 +427,12 @@ ToolChain::getDefaultUnwindTableLevel(const ArgList &Args) 
const {
   return UnwindTableLevel::None;
 }
 
+unsigned ToolChain::GetDefaultDwarfVersion() const {
+  // TODO: Remove the RISC-V special case when R_RISCV_SET_ULEB128 linker
+  // support becomes more widely available.
+  return getTriple().isRISCV() ? 4 : 5;
+}
+
 Tool *ToolChain::getClang() const {
   if (!Clang)
 Clang.reset(new tools::Clang(*this, useIntegratedBackend()));

diff  --git a/clang/test/Driver/clang-g-opts.c 
b/clang/test/Driver/clang-g-opts.c
index d982b1070cae11..5ee0fe64fe484f 100644
--- a/clang/test/Driver/clang-g-opts.c
+++ b/clang/test/Driver/clang-g-opts.c
@@ -37,3 +37,8 @@
 
 // CHECK-WITH-G-STANDALONE: "-debug-info-kind=standalone"
 // CHECK-WITH-G-STANDALONE: "-dwarf-version=2"
+
+/// TODO: Special case before R_RISCV_SET_ULEB128 linker support becomes more 
widely available.
+// RUN: %clang -### -S %s -g --target=riscv64-linux-gnu 2>&1 | FileCheck 
--check-prefix=VERSION4 %s
+// RUN: %clang -### -S %s -g --target=riscv64-unknown-elf 2>&1 | FileCheck 
--check-prefix=VERSION4 %s
+// VERSION4: "-dwarf-version=4"



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

[PATCH] D157663: [Driver] Default riscv*- triples to -fdebug-default-version=4

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbbc0f99f3bc9: [Driver] Default riscv*- triples to 
-fdebug-default-version=4 (authored by MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D157663?vs=549233&id=549956#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157663

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/clang-g-opts.c


Index: clang/test/Driver/clang-g-opts.c
===
--- clang/test/Driver/clang-g-opts.c
+++ clang/test/Driver/clang-g-opts.c
@@ -37,3 +37,8 @@
 
 // CHECK-WITH-G-STANDALONE: "-debug-info-kind=standalone"
 // CHECK-WITH-G-STANDALONE: "-dwarf-version=2"
+
+/// TODO: Special case before R_RISCV_SET_ULEB128 linker support becomes more 
widely available.
+// RUN: %clang -### -S %s -g --target=riscv64-linux-gnu 2>&1 | FileCheck 
--check-prefix=VERSION4 %s
+// RUN: %clang -### -S %s -g --target=riscv64-unknown-elf 2>&1 | FileCheck 
--check-prefix=VERSION4 %s
+// VERSION4: "-dwarf-version=4"
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -427,6 +427,12 @@
   return UnwindTableLevel::None;
 }
 
+unsigned ToolChain::GetDefaultDwarfVersion() const {
+  // TODO: Remove the RISC-V special case when R_RISCV_SET_ULEB128 linker
+  // support becomes more widely available.
+  return getTriple().isRISCV() ? 4 : 5;
+}
+
 Tool *ToolChain::getClang() const {
   if (!Clang)
 Clang.reset(new tools::Clang(*this, useIntegratedBackend()));
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -561,7 +561,7 @@
 
   // Return the DWARF version to emit, in the absence of arguments
   // to the contrary.
-  virtual unsigned GetDefaultDwarfVersion() const { return 5; }
+  virtual unsigned GetDefaultDwarfVersion() const;
 
   // Some toolchains may have different restrictions on the DWARF version and
   // may need to adjust it. E.g. NVPTX may need to enforce DWARF2 even when 
host


Index: clang/test/Driver/clang-g-opts.c
===
--- clang/test/Driver/clang-g-opts.c
+++ clang/test/Driver/clang-g-opts.c
@@ -37,3 +37,8 @@
 
 // CHECK-WITH-G-STANDALONE: "-debug-info-kind=standalone"
 // CHECK-WITH-G-STANDALONE: "-dwarf-version=2"
+
+/// TODO: Special case before R_RISCV_SET_ULEB128 linker support becomes more widely available.
+// RUN: %clang -### -S %s -g --target=riscv64-linux-gnu 2>&1 | FileCheck --check-prefix=VERSION4 %s
+// RUN: %clang -### -S %s -g --target=riscv64-unknown-elf 2>&1 | FileCheck --check-prefix=VERSION4 %s
+// VERSION4: "-dwarf-version=4"
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -427,6 +427,12 @@
   return UnwindTableLevel::None;
 }
 
+unsigned ToolChain::GetDefaultDwarfVersion() const {
+  // TODO: Remove the RISC-V special case when R_RISCV_SET_ULEB128 linker
+  // support becomes more widely available.
+  return getTriple().isRISCV() ? 4 : 5;
+}
+
 Tool *ToolChain::getClang() const {
   if (!Clang)
 Clang.reset(new tools::Clang(*this, useIntegratedBackend()));
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -561,7 +561,7 @@
 
   // Return the DWARF version to emit, in the absence of arguments
   // to the contrary.
-  virtual unsigned GetDefaultDwarfVersion() const { return 5; }
+  virtual unsigned GetDefaultDwarfVersion() const;
 
   // Some toolchains may have different restrictions on the DWARF version and
   // may need to adjust it. E.g. NVPTX may need to enforce DWARF2 even when host
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] d7fcb5b - clang: Don't use grep in a test

[Matt Arsenault via cfe-commits]

Author: Matt Arsenault
Date: 2023-08-14T11:28:41-04:00
New Revision: d7fcb5b6b5280dc61d5afe0920bb78a82cfe2f27

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

LOG: clang: Don't use grep in a test

Issue #10894 seems to claim this wasn't working. The test does seem to
work as intended, except the CHECKs added in
3ac4299d3798eb7078905d5fc8f23781556c90a1 aren't doing anything since
it wasn't really using FileCheck.

Added: 


Modified: 
clang/test/CodeGen/2007-06-15-AnnotateAttribute.c

Removed: 




diff  --git a/clang/test/CodeGen/2007-06-15-AnnotateAttribute.c 
b/clang/test/CodeGen/2007-06-15-AnnotateAttribute.c
index 05a9e6d7dc3941..7482fad190a4c3 100644
--- a/clang/test/CodeGen/2007-06-15-AnnotateAttribute.c
+++ b/clang/test/CodeGen/2007-06-15-AnnotateAttribute.c
@@ -1,5 +1,13 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - | grep llvm.global.annotations
-// RUN: %clang_cc1 -emit-llvm %s -o - | grep llvm.var.annotation | count 3
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: @.str.3 = private unnamed_addr constant [28 x i8] 
c"GlobalValAnnotationWithArgs\00", section "llvm.metadata"
+// CHECK-NEXT: @.args = private unnamed_addr constant { i32, 
%struct.TestStruct } { i32 42, %struct.TestStruct { i32 1, i32 2 } }, section 
"llvm.metadata"
+
+// CHECK: llvm.global.annotations
+
+// CHECK: llvm.var.annotation
+// CHECK: llvm.var.annotation
+// CHECK: llvm.var.annotation
 
 /* Global variable with attribute */
 int X __attribute__((annotate("GlobalValAnnotation")));
@@ -20,13 +28,11 @@ struct TestStruct {
   int b;
 };
 int Y __attribute__((annotate(
-  "GlobalValAnnotationWithArgs", 
+  "GlobalValAnnotationWithArgs",
   42,
   (struct TestStruct) { .a = 1, .b = 2 }
 )));
 
-// CHECK: @.str.3 = private unnamed_addr constant [28 x i8] 
c"GlobalValAnnotationWithArgs\00", section "llvm.metadata"
-// CHECK-NEXT: @.args = private unnamed_addr constant { i32, 
%struct.TestStruct } { i32 42, %struct.TestStruct { i32 1, i32 2 } }, section 
"llvm.metadata"
 
 int main(void) {
   static int a __attribute__((annotate("GlobalValAnnotation")));



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

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: ldionne.
aaron.ballman added inline comments.



Comment at: clang/lib/Headers/stddef.h:16
+defined(__need_wint_t) ||  
\
+(defined(__STDC_WANT_LIB_EXT1__) && __STDC_WANT_LIB_EXT1__ >= 1)
 

C23 K.3.1.1p2: The functions, macros, and types declared or defined in K.3 and 
its subclauses are declared and defined by their respective headers if 
__STDC_WANT_LIB_EXT1__ is defined as a macro which expands to the integer 
constant 1 at the point in the source file where the appropriate header is 
first included.

So I don't think this should be `>= 1`. Same below. (Though I notice we seem to 
do this in other places, so perhaps that's more of a general cleanup we should 
make?)



Comment at: clang/lib/Headers/stddef.h:118-122
+#ifdef __cplusplus
+namespace std {
+typedef decltype(nullptr) nullptr_t;
+}
+using ::std::nullptr_t;

Related:

https://github.com/llvm/llvm-project/issues/37564
https://cplusplus.github.io/LWG/issue3484

CC @ldionne



Comment at: clang/test/Headers/stddef.c:1-3
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c99 
-std=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c11 
-std=c11 %s
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c23 
-std=c23 %s

Are the triples necessary? Better if we can remove them.



Comment at: clang/test/Headers/stddef.c:7
+
+ptrdiff_t p0; // c99-error{{unknown}} c11-error{{unknown}} c23-error{{unknown}}
+size_t s0; // c99-error{{unknown}} c11-error{{unknown}} c23-error{{unknown}}

At least one of these `unknown` errors should be fully spelled out (the rest 
can then be assumed to be the same wording as the long-form).



Comment at: clang/test/Headers/stddefneeds.c:1-2
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c99 
-std=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c23 
-std=c23 %s
+

Do we need the triples?



Comment at: clang/test/Headers/stddefneeds.c:8
+
+ptrdiff_t p0; // c99-error{{unknown}} c23-error{{unknown}}
+size_t s0; // c99-error{{unknown}} c23-error{{unknown}}

Should spell out one of these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157757

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

[PATCH] D157869: [clang-tidy] Avoid overflow when dumping unsigned integer values

[Daniel Alcaide Nombela via Phabricator via cfe-commits]

ealcdan added a comment.

Thanks for the review. How should I procceed? Should I remove the "Fixes:" 
entry from the commit message?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157869

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

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D157632#4581219 , @ellis wrote:

> In D157632#4580576 , @zequanwu 
> wrote:
>
>> BTW, I noticed something strange with `-pgo-function-entry-coverage` when 
>> merging via llvm-profdata.
>
> This is intentional. The two raw profiles individually store blocks as either 
> covered or not, but when we merge them they get converted to counters and 
> accumulated.
> https://github.com/llvm/llvm-project/blob/6a0feb1503e21432e63d93b44357bad43f8026d1/llvm/lib/ProfileData/InstrProf.cpp#L726
> Then in `PGOUseFunc::populateCoverage()` we annotate blocks as alive if their 
> counter value is non-zero.
> https://github.com/llvm/llvm-project/blob/1aee2434ce4c9b5785cbc8f72cbbbd64f9e85297/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp#L1399
> My logic was that the indexed profile represents these counters as ints, so 
> we might as well accumulate them. Also, this makes the implementation simpler.

Thanks for explanation. That makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

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

[PATCH] D157554: [NFC][Clang] Fix static analyzer concern about null pointer dereference

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews updated this revision to Diff 549960.
eandrews added a comment.

Applied review comments


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

https://reviews.llvm.org/D157554

Files:
  clang/lib/Sema/SemaExprCXX.cpp


Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -9072,8 +9072,10 @@
 MultiLevelTemplateArgumentList MLTAL(Param, TAL.asArray(),
  /*Final=*/false);
 MLTAL.addOuterRetainedLevels(TPL->getDepth());
-Expr *IDC = Param->getTypeConstraint()->getImmediatelyDeclaredConstraint();
-ExprResult Constraint = SubstExpr(IDC, MLTAL);
+const TypeConstraint *TC = Param->getTypeConstraint();
+assert(TC && "Type Constraint cannot be null here");
+ExprResult Constraint =
+SubstExpr(TC->getImmediatelyDeclaredConstraint(), MLTAL);
 if (Constraint.isInvalid()) {
   Status = concepts::ExprRequirement::SS_ExprSubstitutionFailure;
 } else {


Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -9072,8 +9072,10 @@
 MultiLevelTemplateArgumentList MLTAL(Param, TAL.asArray(),
  /*Final=*/false);
 MLTAL.addOuterRetainedLevels(TPL->getDepth());
-Expr *IDC = Param->getTypeConstraint()->getImmediatelyDeclaredConstraint();
-ExprResult Constraint = SubstExpr(IDC, MLTAL);
+const TypeConstraint *TC = Param->getTypeConstraint();
+assert(TC && "Type Constraint cannot be null here");
+ExprResult Constraint =
+SubstExpr(TC->getImmediatelyDeclaredConstraint(), MLTAL);
 if (Constraint.isInvalid()) {
   Status = concepts::ExprRequirement::SS_ExprSubstitutionFailure;
 } else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157793: [Headers] Add missing __need_ macros to stdarg.h

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D157793#4583698 , @iana wrote:

> In D157793#4583663 , @MaskRay wrote:
>
>>> Apple needs a __need_ macro for va_list. Add one, and also ones for 
>>> va_start/va_arg/va_end, __va_copy, va_copy.
>>
>> Do you have a link to the specification or the source that this is needed?
>
> We need is so that our  doesn't have to redeclare 
> va_list and doesn't pull in all of stdarg.h either. As far as I know there's 
> no specification for this (or stddef.h) being include-able in pieces.

I'm not opposed, but this adds significant complexity to support a questionable 
use case -- the C standard library headers are not intended to be partially 
included, so if the plan is to use these `__need` macros in all of the headers 
we provide, I think that should go through an RFC process to be sure we want to 
maintain the added complexity. (Also, who is "we" in this case?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157793

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

[PATCH] D157855: [clang][ExprConstant] Improve error message of compound assignment against uninitialized object

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

Generally LGTM, but I feel like the changes aren't c++2a specific (except for 
uninitialized variables in constexpr functions).




Comment at: clang/lib/AST/ExprConstant.cpp:4447
+  Info.FFDiag(E, diag::note_constexpr_access_uninit)
+  << /*read of*/ 0 << /*uninitialized object*/ 1;
+  return false;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157855

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

[PATCH] D157888: [NFC][Clang] Fix static analyzer concern about null value dereference

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews created this revision.
eandrews added reviewers: aaron.ballman, tahonermann.
Herald added subscribers: steakhal, abrachet, manas, ASDenysPetrov, martong, 
dkrupp, donat.nagy, Szelethus, a.sidorin, baloghadamsoftware.
Herald added a reviewer: NoQ.
Herald added a project: All.
eandrews requested review of this revision.

https://reviews.llvm.org/D157888

Files:
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -653,10 +653,11 @@
   if (Type.isSuppressOnSink()) {
 const ExplodedNode *AcquireNode = getAcquireSite(ErrorNode, Sym, C);
 if (AcquireNode) {
+  const Stmt *S = AcquireNode->getStmtForDiagnostics();
+  assert(S && "Statmement cannot be null.");
   PathDiagnosticLocation LocUsedForUniqueing =
   PathDiagnosticLocation::createBegin(
-  AcquireNode->getStmtForDiagnostics(), C.getSourceManager(),
-  AcquireNode->getLocationContext());
+  S, C.getSourceManager(), AcquireNode->getLocationContext());
 
   R = std::make_unique(
   Type, Msg, ErrorNode, LocUsedForUniqueing,


Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -653,10 +653,11 @@
   if (Type.isSuppressOnSink()) {
 const ExplodedNode *AcquireNode = getAcquireSite(ErrorNode, Sym, C);
 if (AcquireNode) {
+  const Stmt *S = AcquireNode->getStmtForDiagnostics();
+  assert(S && "Statmement cannot be null.");
   PathDiagnosticLocation LocUsedForUniqueing =
   PathDiagnosticLocation::createBegin(
-  AcquireNode->getStmtForDiagnostics(), C.getSourceManager(),
-  AcquireNode->getLocationContext());
+  S, C.getSourceManager(), AcquireNode->getLocationContext());
 
   R = std::make_unique(
   Type, Msg, ErrorNode, LocUsedForUniqueing,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157280: [PGO] Enable `-fprofile-update` for `-fprofile-generate`

[Qiongsi Wu via Phabricator via cfe-commits]

qiongsiwu1 added a comment.

Ping for review. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157280

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

[PATCH] D157885: [NFC][Clang] Fix static analyzer concern about null value derefence

[Tom Honermann via Phabricator via cfe-commits]

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

Looks fine to me!


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

https://reviews.llvm.org/D157885

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

[PATCH] D157554: [NFC][Clang] Fix static analyzer concern about null pointer dereference

[Tom Honermann via Phabricator via cfe-commits]

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

Looks good to me!


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

https://reviews.llvm.org/D157554

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

[PATCH] D157888: [NFC][Clang] Fix static analyzer concern about null value dereference

[Tom Honermann via Phabricator via cfe-commits]

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

Looks good to me!


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

https://reviews.llvm.org/D157888

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

[PATCH] D157691: [ASTImporter] Remove extranous FunctionTemplateDecl introduced by templated friend

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

It is better to add a test to ASTImporterTests.cpp with the same code, and 
check the properties of DeclContext and LexicalDeclContext in the From and To 
AST (should be the same in "From" and "To" side).




Comment at: clang/lib/AST/ASTImporter.cpp:6451
+  if (D->getFriendObjectKind() == Decl::FOK_None)
+LexicalDC->addDeclInternal(ToFunc);
 

`addDeclToContexts(D, ToFunction)` should be better


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157691

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

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy marked 13 inline comments as done.
donat.nagy added a comment.

I handled the inline comments, I'll check the example code and edit the release 
notes tomorrow.




Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:272
+  if (const auto ConcreteRight = Right.getAs()) {
+const std::int64_t RHS = ConcreteRight->getValue().getExtValue();
+assert(RHS > MaximalAllowedShift);

steakhal wrote:
> `getExtValue()`, I believe, asserts that it "fits", thus the concrete int is 
> at most 64 bits wide. Does it work for `_BigInt`s? (or whatever we have like 
> that :D)
This final check is executed when we already know that 0 <= ConcreteRight < 128 
//(or the bit width of the largest integer type)//. I added a comment that 
mentions this and replaced the type `std::int64_t` with a plain `unsigned`.

I also updated the testcase `large_right` to ensure that later changes don't 
introduce crashes related to absurdly oversized right arguments.



Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:335-339
+auto R =
+std::make_unique(BT, ShortMsg, Msg, ErrNode);
+bugreporter::trackExpressionValue(ErrNode, Op->getLHS(), *R);
+bugreporter::trackExpressionValue(ErrNode, Op->getRHS(), *R);
+return R;

steakhal wrote:
> AHA! Now I got you. You also call this `R`.
> Just use `R` at the callsite as well. Job done.
This is a code fragment that survived my refactoring efforts without 
significant changes... until now :)

Now that you highlighted it, I'm renaming `R` to `BR` just to be a contrarian :)



Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:355-357
+if (!BTPtr)
+  BTPtr = std::make_unique(this, "Bitwise shift",
+"Suspicious operation");

steakhal wrote:
> How about eagerly inline initializing this field? And avoiding `mutable` as 
> well.
This pattern systematically appears in all checkers that I've seen because the 
constructor of `BugType` queries and stores the name of the checker, which is 
only initialized when the checker (`*this`) is registered. This dependency 
means that the `BugType` cannot be initialized in the constructor of the 
`Checker`.



Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:53
+class BitwiseShiftValidator {
+  // Primary mutable state:
+  CheckerContext &Ctx;

steakhal wrote:
> donat.nagy wrote:
> > steakhal wrote:
> > > Where does this comment corresponds to?
> > The two data members (`Ctx` and `State`) that are directly below the 
> > comment.
> > 
> > I tried to split the list of data members into three groups (primary 
> > mutable state, secondary mutable state, `const`  members), but now I 
> > reconsider this I feel that these header comments are actually superfluous.
> > 
> > I'll return to a variant of the earlier layout where I put 
> > `NonNegFlags`/`UpperBound` to the end and comment that they are only used 
> > for note tag creation; but don't emphasize the (immediately visible) 
> > `const`ness / non-`const`-ness of other members with comments.
> One way to emphasize this is by choosing names like `FoldedState`, or 
> anything else that has something to do with "motion" or "change".
> Given that if we have `ProgramStateRefs` in helper classes they usually 
> remain still. Think of BugReportVisitors for example. Consequently,it's 
> important to raise awareness when it's not like that. A different name would 
> achieve this.
My intuition is that the variable name "State" already implies that it will be 
regularly changed and updated as we are going forward; but you're right that in 
this project we've a tradition of storing snapshots of "old" states in various 
helper classes.

(By the way, I'm a bit worried when I read code that stores and passes around 
random stale snapshots of the state. Of course we cannot avoid the branching 
nature of the exploded graph, but other than that it'd be good to store the 
State in language constructs where you cannot accidentally lose information by 
building onto an older state variable.) 



Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:60-63
+  // If there is an upper bound, it is <= the size of a primitive type
+  // (expressed in bits), so this is a very safe sentinel value:
+  enum { NoUpperBound = 999 };
+  unsigned UpperBound = NoUpperBound;

steakhal wrote:
> donat.nagy wrote:
> > steakhal wrote:
> > > Have you considered using `std::optional`?
> > > Given that the dimension of this variable is "bits", I think it would be 
> > > great to have that in its name.
> > I considered using `std::optional`, but using this "old-school" solution 
> > ensures that `NoUpperBound` is comparable to and larger than the "real" 
> > upper bound values, which lets me avoid some ugly boolean logic. I'll 
> > u

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy updated this revision to Diff 549978.
donat.nagy marked 7 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156312

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/analyzer-enabled-checkers.c
  clang/test/Analysis/bitwise-ops-nocrash.c
  clang/test/Analysis/bitwise-ops.c
  clang/test/Analysis/bitwise-shift-common.c
  clang/test/Analysis/bitwise-shift-pedantic.c
  clang/test/Analysis/bitwise-shift-sanity-checks.c
  clang/test/Analysis/bitwise-shift-state-update.c
  clang/test/Analysis/casts.c
  clang/test/Analysis/diagnostics/track_subexpressions.cpp
  clang/test/Analysis/left-shift-cxx2a.cpp
  clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
  clang/test/Analysis/symbol-simplification-nonloc-loc.cpp

Index: clang/test/Analysis/symbol-simplification-nonloc-loc.cpp
===
--- clang/test/Analysis/symbol-simplification-nonloc-loc.cpp
+++ clang/test/Analysis/symbol-simplification-nonloc-loc.cpp
@@ -14,7 +14,7 @@
   if (p) {
 // no-crash
   }
-  if (p == (int *)0x404) {
+  if (p == (int *)0x1b) {
 // no-crash
   }
 }
@@ -29,7 +29,7 @@
   if (p) {
 // no-crash
   }
-  if (p == (int *)0x404) {
+  if (p == (int *)0x1b) {
 // no-crash
   }
 }
@@ -43,8 +43,6 @@
   nonloc_OP_loc(p, BINOP(-)); // no-crash
 
   // Bitwise operators:
-  // expected-warning@+2 {{The result of the left shift is undefined due to shifting by '1028', which is greater or equal to the width of type 'int' [core.UndefinedBinaryOperatorResult]}}
-  // expected-warning@+2 {{The result of the right shift is undefined due to shifting by '1028', which is greater or equal to the width of type 'int' [core.UndefinedBinaryOperatorResult]}}
   nonloc_OP_loc(p, BINOP(<<)); // no-crash
   nonloc_OP_loc(p, BINOP(>>)); // no-crash
   nonloc_OP_loc(p, BINOP(&));
Index: clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
===
--- clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
+++ clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
@@ -24,6 +24,7 @@
 // CHECK-NEXT: apiModeling.TrustReturnsNonnull
 // CHECK-NEXT: apiModeling.llvm.CastValue
 // CHECK-NEXT: apiModeling.llvm.ReturnValue
+// CHECK-NEXT: core.BitwiseShift
 // CHECK-NEXT: core.DivideZero
 // CHECK-NEXT: core.DynamicTypePropagation
 // CHECK-NEXT: core.NonnilStringConstants
Index: clang/test/Analysis/left-shift-cxx2a.cpp
===
--- clang/test/Analysis/left-shift-cxx2a.cpp
+++ clang/test/Analysis/left-shift-cxx2a.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify=expected,cxx17 -std=c++17 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify=expected,cxx2a -std=c++2a %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-disable-checker core.BitwiseShift -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify=expected,cxx17 -std=c++17 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-disable-checker core.BitwiseShift -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify=expected,cxx2a -std=c++2a %s
 
 int testNegativeShift(int a) {
   if (a == -5) {
Index: clang/test/Analysis/diagnostics/track_subexpressions.cpp
===
--- clang/test/Analysis/diagnostics/track_subexpressions.cpp
+++ clang/test/Analysis/diagnostics/track_subexpressions.cpp
@@ -12,10 +12,10 @@
 
 void shift_by_undefined_value() {
   uint8_t shift_amount = get_uint8_max(); // expected-note{{'shift_amount' initialized to 255}}
-// expected-note@-1{{Calling 'get_uint8_max'}}
-// expected-note@-2{{Returning from 'get_uint8_max'}}
-  (void)(TCP_MAXWIN << shift_amount); // expected-warning{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}}
-  // expected-note@-1{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}}
+  // expected-note@-1{{Calling 'get_uint8_max'}}
+  // expected-note@-2{{Returning from 'get_uint8_max'}}
+  (void)(TCP_MAXWIN << shift_amount); // expected-warning{{Left shift by '255' overflows the capacity of 'int'}}
+  // expected-note@-1{{The result of left shift

[PATCH] D155081: Specify the developer policy around links to external resources

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 549979.
aaron.ballman added a comment.

Tweaked wording to link to the policy about adding links to github issues in 
commit messages instead.


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

https://reviews.llvm.org/D155081

Files:
  llvm/docs/DeveloperPolicy.rst


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -201,6 +201,11 @@
   entire failing program into ``llvm/test`` as this creates a *time-to-test*
   burden on all developers. Please keep them short.
 
+* Avoid adding links to resources that are not available to the entire
+  community, such as links to private bug trackers, internal corporate
+  documentation, etc. Instead, add sufficient comments to the test to provide
+  the context behind such links.
+
 Note that llvm/test and clang/test are designed for regression and small 
feature
 tests only. More extensive test cases (e.g., entire applications, benchmarks,
 etc) should be added to the ``llvm-test`` test suite.  The llvm-test suite is
@@ -256,6 +261,11 @@
the change (more invasive changes require more testing). A reasonable subset
might be something like "``llvm-test/MultiSource/Benchmarks``".
 
+#. Ensure that links in source code and test files point to publicly available
+   resources and are used primarily to add additional information rather than
+   to supply critical context. The surrounding comments should be sufficient
+   to provide the context behind such links.
+
 Additionally, the committer is responsible for addressing any problems found in
 the future that the change is responsible for.  For example:
 
@@ -336,8 +346,6 @@
   code snippets and gory details should be left to bug comments, web
   review or the mailing list.
 
-* If the patch fixes a bug in GitHub Issues, please include the PR# in the 
message.
-
 * Text formatting and spelling should follow the same rules as documentation
   and in-code comments, ex. capitalization, full stop, etc.
 
@@ -346,8 +354,13 @@
   related commit. This could be as simple as "Revert commit  because it
   caused PR#".
 
-* If the patch has been reviewed, add a link to its review page, as shown
-  `here `_.
+* It is acceptable to add metadata to the commit message to automate processes,
+  including for downstream consumers. If the patch fixes a bug in GitHub 
Issues,
+  we encourage adding a reference to the issue being closed, as described
+  `here `_.
+  Other kinds of metadata are also acceptable, including links to resources
+  that are not available to the entire community. However, such links should
+  not be used in place of making the commit message self-explanatory.
 
 For minor violations of these recommendations, the community normally favors
 reminding the contributor of this policy over reverting. Minor corrections and


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -201,6 +201,11 @@
   entire failing program into ``llvm/test`` as this creates a *time-to-test*
   burden on all developers. Please keep them short.
 
+* Avoid adding links to resources that are not available to the entire
+  community, such as links to private bug trackers, internal corporate
+  documentation, etc. Instead, add sufficient comments to the test to provide
+  the context behind such links.
+
 Note that llvm/test and clang/test are designed for regression and small feature
 tests only. More extensive test cases (e.g., entire applications, benchmarks,
 etc) should be added to the ``llvm-test`` test suite.  The llvm-test suite is
@@ -256,6 +261,11 @@
the change (more invasive changes require more testing). A reasonable subset
might be something like "``llvm-test/MultiSource/Benchmarks``".
 
+#. Ensure that links in source code and test files point to publicly available
+   resources and are used primarily to add additional information rather than
+   to supply critical context. The surrounding comments should be sufficient
+   to provide the context behind such links.
+
 Additionally, the committer is responsible for addressing any problems found in
 the future that the change is responsible for.  For example:
 
@@ -336,8 +346,6 @@
   code snippets and gory details should be left to bug comments, web
   review or the mailing list.
 
-* If the patch fixes a bug in GitHub Issues, please include the PR# in the message.
-
 * Text formatting and spelling should follow the same rules as documentation
   and in-code comments, ex. capitalization, full stop, etc.
 
@@ -346,8 +354,13 @@
   related commit. This could be as simple as "Revert commit  because it
   caused PR#".
 
-* If the patch has 

[clang] 0c3a02b - Function multi-versioning: disable ifunc for ELF targets other than glibc/Android/FreeBSD

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2023-08-14T08:59:59-07:00
New Revision: 0c3a02b8c09bb408a74a638a263e51d67c92ca74

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

LOG: Function multi-versioning: disable ifunc for ELF targets other than 
glibc/Android/FreeBSD

Generalize D127933 (Fuchsia special case) to other ELF targets. Ensure
that musl, NetBSD, OpenBSD, etc do not get ifunc codegen which is
unsupported in their rtld.

Link: 
https://discourse.llvm.org/t/does-ifunc-use-from-llvm-require-os-support/67628
Close: https://github.com/llvm/llvm-project/issues/64631

Added: 


Modified: 
clang/include/clang/Basic/TargetInfo.h
clang/test/CodeGen/attr-target-mv-va-args.c
clang/test/CodeGen/unique-internal-linkage-names.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/TargetInfo.h 
b/clang/include/clang/Basic/TargetInfo.h
index 41ef47eb565b1c..61be52149341f0 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1414,7 +1414,9 @@ class TargetInfo : public TransferrableTargetInfo,
 
   /// Identify whether this target supports IFuncs.
   bool supportsIFunc() const {
-return getTriple().isOSBinFormatELF() && !getTriple().isOSFuchsia();
+return getTriple().isOSBinFormatELF() &&
+   ((getTriple().isOSLinux() && !getTriple().isMusl()) ||
+getTriple().isOSFreeBSD());
   }
 
   // Validate the contents of the __builtin_cpu_supports(const char*)

diff  --git a/clang/test/CodeGen/attr-target-mv-va-args.c 
b/clang/test/CodeGen/attr-target-mv-va-args.c
index e75796d7ee0383..96821c610235bd 100644
--- a/clang/test/CodeGen/attr-target-mv-va-args.c
+++ b/clang/test/CodeGen/attr-target-mv-va-args.c
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s 
--check-prefix=LINUX
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s 
--check-prefix=IFUNC-ELF
+// RUN: %clang_cc1 -triple x86_64-pc-freebsd -emit-llvm %s -o - | FileCheck %s 
--check-prefix=IFUNC-ELF
 // RUN: %clang_cc1 -triple x86_64-windows-pc -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=NO-IFUNC,WINDOWS
-// RUN: %clang_cc1 -triple x86_64-fuchsia -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=NO-IFUNC,FUCHSIA
+// RUN: %clang_cc1 -triple x86_64-linux-musl -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=NO-IFUNC,NO-IFUNC-ELF
+// RUN: %clang_cc1 -triple x86_64-fuchsia -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=NO-IFUNC,NO-IFUNC-ELF
 int __attribute__((target("sse4.2"))) foo(int i, ...) { return 0; }
 int __attribute__((target("arch=sandybridge"))) foo(int i, ...);
 int __attribute__((target("arch=ivybridge"))) foo(int i, ...) {return 1;}
@@ -10,23 +12,23 @@ int bar(void) {
   return foo(1, 'a', 1.1) + foo(2, 2.2, "asdf");
 }
 
-// LINUX: @foo.ifunc = weak_odr ifunc i32 (i32, ...), ptr @foo.resolver
-// LINUX: define{{.*}} i32 @foo.sse4.2(i32 noundef %i, ...)
-// LINUX: ret i32 0
-// LINUX: define{{.*}} i32 @foo.arch_ivybridge(i32 noundef %i, ...)
-// LINUX: ret i32 1
-// LINUX: define{{.*}} i32 @foo(i32 noundef %i, ...)
-// LINUX: ret i32 2
-// LINUX: define{{.*}} i32 @bar()
-// LINUX: call i32 (i32, ...) @foo.ifunc(i32 noundef 1, i32 noundef 97, double
-// LINUX: call i32 (i32, ...) @foo.ifunc(i32 noundef 2, double noundef 
2.2{{[0-9Ee+]+}}, ptr noundef
+// IFUNC-ELF: @foo.ifunc = weak_odr ifunc i32 (i32, ...), ptr @foo.resolver
+// IFUNC-ELF: define{{.*}} i32 @foo.sse4.2(i32 noundef %i, ...)
+// IFUNC-ELF: ret i32 0
+// IFUNC-ELF: define{{.*}} i32 @foo.arch_ivybridge(i32 noundef %i, ...)
+// IFUNC-ELF: ret i32 1
+// IFUNC-ELF: define{{.*}} i32 @foo(i32 noundef %i, ...)
+// IFUNC-ELF: ret i32 2
+// IFUNC-ELF: define{{.*}} i32 @bar()
+// IFUNC-ELF: call i32 (i32, ...) @foo.ifunc(i32 noundef 1, i32 noundef 97, 
double
+// IFUNC-ELF: call i32 (i32, ...) @foo.ifunc(i32 noundef 2, double noundef 
2.2{{[0-9Ee+]+}}, ptr noundef
 
-// LINUX: define weak_odr ptr @foo.resolver() comdat
-// LINUX: ret ptr @foo.arch_sandybridge
-// LINUX: ret ptr @foo.arch_ivybridge
-// LINUX: ret ptr @foo.sse4.2
-// LINUX: ret ptr @foo
-// LINUX: declare i32 @foo.arch_sandybridge(i32 noundef, ...)
+// IFUNC-ELF: define weak_odr ptr @foo.resolver() comdat
+// IFUNC-ELF: ret ptr @foo.arch_sandybridge
+// IFUNC-ELF: ret ptr @foo.arch_ivybridge
+// IFUNC-ELF: ret ptr @foo.sse4.2
+// IFUNC-ELF: ret ptr @foo
+// IFUNC-ELF: declare i32 @foo.arch_sandybridge(i32 noundef, ...)
 
 // NO-IFUNC: define dso_local i32 @foo.sse4.2(i32 noundef %i, ...)
 // NO-IFUNC: ret i32 0
@@ -39,10 +41,10 @@ int bar(void) {
 // NO-IFUNC: call i32 (i32, ...) @foo.resolver(i32 noundef 2, double noundef 
2.2{{[0-9Ee+]+}}, ptr noundef
 
 // WINDOWS: define weak_odr dso_local i32 @foo.resolver(i32 %0, ...) comdat
-// FUCHSIA: define

[PATCH] D157080: [clangd] Symbol search includes parameter types for (member) functions

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

I understood https://github.com/clangd/clangd/issues/1344 as being about 
`workspace/symbol` rather than `textDocument/documentSymbol`, though I see now 
that both are affected.

The analysis is a bit different for the two, though: unlike `DocumentSymbol`, 
the result types for `workspace/symbol` (LSP specifies two options, 
`SymbolInformation` or `WorkspaceSymbol`) do not have a `detail` field. So, for 
`workspace/symbol`, the issue would require a fix on the clangd side (or a 
change to LSP, I guess).

@sammccall, would you support adding the signature to the symbol name for 
`workspace/symbol`?


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

https://reviews.llvm.org/D157080

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

[clang] 989ce06 - [clang][WebAssembly] Link crt1 even in case of -shared

[Sam Clegg via cfe-commits]

Author: YAMAMOTO Takashi
Date: 2023-08-14T09:36:44-07:00
New Revision: 989ce069a4638fd561bebc95f478ca9e45154fec

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

LOG: [clang][WebAssembly] Link crt1 even in case of -shared

This allows -mexec-model=reactor -shared produces a library module
with _initialize entrypoint, which is preferrable over __wasm_call_ctors.

This partially reverts https://reviews.llvm.org/D153293

Discussion: https://github.com/dicej/component-linking-demo/issues/3

Reviewed By: sbc100

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/WebAssembly.cpp
clang/test/Driver/wasm-toolchain.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp 
b/clang/lib/Driver/ToolChains/WebAssembly.cpp
index 36bed3166ff3c6..2098a68cbc6e56 100644
--- a/clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -77,31 +77,43 @@ void wasm::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
   Args.AddAllArgs(CmdArgs, options::OPT_u);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
-  const char *Crt1 = "crt1.o";
+  bool IsCommand = true;
+  const char *Crt1;
   const char *Entry = nullptr;
 
-  // If crt1-command.o exists, it supports new-style commands, so use it.
-  // Otherwise, use the old crt1.o. This is a temporary transition measure.
-  // Once WASI libc no longer needs to support LLVM versions which lack
-  // support for new-style command, it can make crt1.o the same as
-  // crt1-command.o. And once LLVM no longer needs to support WASI libc
-  // versions before that, it can switch to using crt1-command.o.
-  if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
-Crt1 = "crt1-command.o";
+  // When -shared is specified, use the reactor exec model unless
+  // specified otherwise.
+  if (Args.hasArg(options::OPT_shared))
+IsCommand = false;
 
   if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
 StringRef CM = A->getValue();
 if (CM == "command") {
-  // Use default values.
+  IsCommand = true;
 } else if (CM == "reactor") {
-  Crt1 = "crt1-reactor.o";
-  Entry = "_initialize";
+  IsCommand = false;
 } else {
   ToolChain.getDriver().Diag(diag::err_drv_invalid_argument_to_option)
   << CM << A->getOption().getName();
 }
   }
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles, 
options::OPT_shared))
+
+  if (IsCommand) {
+// If crt1-command.o exists, it supports new-style commands, so use it.
+// Otherwise, use the old crt1.o. This is a temporary transition measure.
+// Once WASI libc no longer needs to support LLVM versions which lack
+// support for new-style command, it can make crt1.o the same as
+// crt1-command.o. And once LLVM no longer needs to support WASI libc
+// versions before that, it can switch to using crt1-command.o.
+Crt1 = "crt1.o";
+if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
+  Crt1 = "crt1-command.o";
+  } else {
+Crt1 = "crt1-reactor.o";
+Entry = "_initialize";
+  }
+
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles))
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(Crt1)));
   if (Entry) {
 CmdArgs.push_back(Args.MakeArgString("--entry"));

diff  --git a/clang/test/Driver/wasm-toolchain.c 
b/clang/test/Driver/wasm-toolchain.c
index 8e4e749df01440..f950283ec42aa0 100644
--- a/clang/test/Driver/wasm-toolchain.c
+++ b/clang/test/Driver/wasm-toolchain.c
@@ -33,19 +33,19 @@
 // LINK_KNOWN: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
-// -shared should be passed through to `wasm-ld` and not include crt1.o with a 
known OS.
+// -shared should be passed through to `wasm-ld` and include crt1-reactor.o 
with a known OS.
 
-// RUN: %clang -### -shared --target=wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN: %clang -### -shared -mexec-model=reactor --target=wasm32-wasi 
--sysroot=/foo %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK_KNOWN_SHARED %s
 // LINK_KNOWN_SHARED: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_KNOWN_SHARED: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "-shared" 
"[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK_KNOWN_SHARED: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1-reactor.o" 
"--entry" "_initialize" "-shared" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
-// -shared should be passed through to `wasm-ld` and not include crt1.o with 
an unknown OS.
+// -shared should be passed through to `wasm-ld` and include crt1-

[PATCH] D156205: wasm: link crt1 even in case of -shared

[Sam Clegg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG989ce069a463: [clang][WebAssembly] Link crt1 even in case of 
-shared (authored by yamt, committed by sbc100).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156205

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain.c


Index: clang/test/Driver/wasm-toolchain.c
===
--- clang/test/Driver/wasm-toolchain.c
+++ clang/test/Driver/wasm-toolchain.c
@@ -33,19 +33,19 @@
 // LINK_KNOWN: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
-// -shared should be passed through to `wasm-ld` and not include crt1.o with a 
known OS.
+// -shared should be passed through to `wasm-ld` and include crt1-reactor.o 
with a known OS.
 
-// RUN: %clang -### -shared --target=wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN: %clang -### -shared -mexec-model=reactor --target=wasm32-wasi 
--sysroot=/foo %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK_KNOWN_SHARED %s
 // LINK_KNOWN_SHARED: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_KNOWN_SHARED: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "-shared" 
"[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK_KNOWN_SHARED: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1-reactor.o" 
"--entry" "_initialize" "-shared" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
-// -shared should be passed through to `wasm-ld` and not include crt1.o with 
an unknown OS.
+// -shared should be passed through to `wasm-ld` and include crt1-reactor.o 
with an unknown OS.
 
-// RUN: %clang -### -shared --target=wasm32-unknown-unknown --sysroot=/foo %s 
2>&1 \
+// RUN: %clang -### -shared -mexec-model=reactor 
--target=wasm32-unknown-unknown --sysroot=/foo %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK_UNKNOWN_SHARED %s
 // LINK_UNKNOWN_SHARED: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_UNKNOWN_SHARED: wasm-ld{{.*}}" "-shared" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK_UNKNOWN_SHARED: wasm-ld{{.*}}" "crt1-reactor.o" "--entry" 
"_initialize" "-shared" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with optimization with known OS.
 
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -77,31 +77,43 @@
   Args.AddAllArgs(CmdArgs, options::OPT_u);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
-  const char *Crt1 = "crt1.o";
+  bool IsCommand = true;
+  const char *Crt1;
   const char *Entry = nullptr;
 
-  // If crt1-command.o exists, it supports new-style commands, so use it.
-  // Otherwise, use the old crt1.o. This is a temporary transition measure.
-  // Once WASI libc no longer needs to support LLVM versions which lack
-  // support for new-style command, it can make crt1.o the same as
-  // crt1-command.o. And once LLVM no longer needs to support WASI libc
-  // versions before that, it can switch to using crt1-command.o.
-  if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
-Crt1 = "crt1-command.o";
+  // When -shared is specified, use the reactor exec model unless
+  // specified otherwise.
+  if (Args.hasArg(options::OPT_shared))
+IsCommand = false;
 
   if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
 StringRef CM = A->getValue();
 if (CM == "command") {
-  // Use default values.
+  IsCommand = true;
 } else if (CM == "reactor") {
-  Crt1 = "crt1-reactor.o";
-  Entry = "_initialize";
+  IsCommand = false;
 } else {
   ToolChain.getDriver().Diag(diag::err_drv_invalid_argument_to_option)
   << CM << A->getOption().getName();
 }
   }
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles, 
options::OPT_shared))
+
+  if (IsCommand) {
+// If crt1-command.o exists, it supports new-style commands, so use it.
+// Otherwise, use the old crt1.o. This is a temporary transition measure.
+// Once WASI libc no longer needs to support LLVM versions which lack
+// support for new-style command, it can make crt1.o the same as
+// crt1-command.o. And once LLVM no longer needs to support WASI libc
+// versions before that, it can switch to using crt1-command.o.
+Crt1 = "crt1.o";
+if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
+  Crt1 = "crt1-command.o";
+  } else {
+Crt1 = "crt1-reactor.o";
+Entry = "_initialize";
+  }
+
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles))
 CmdArgs.push_back(Args.MakeArgString(ToolChain

[PATCH] D86993: Document Clang's expectations of the C standard library.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D86993#4584619 , @nikic wrote:

> @aaron.ballman I wanted to check back whether the linked document is what you 
> had in mind, or whether some more/else would be needed.

Sorry about the delayed review; this fell off my radar! This is exactly along 
the lines of what I was thinking of -- I left some comments on the document. In 
terms of next steps, I think we can do a few things:

1. You and I can iterate on the document until we're ready to submit it to WG14
2. Simultaneously, I think we can move forward with this review -- we can add a 
statement along the lines of "C standard library implementations that do not 
guarantee these properties are incompatible with Clang and LLVM (and with 
several other major compilers); please see WG14 N, which proposes changes 
to the C standard to make this behavior conforming." (I'm not strongly tied to 
these words, just so long as there's some link back to the WG14 document 
justifying the deviation from the C standard.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86993

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

[PATCH] D155081: Specify the developer policy around links to external resources

[Tanya Lattner via Phabricator via cfe-commits]

tonic added a comment.

@lattner Please review.


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

https://reviews.llvm.org/D155081

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

[PATCH] D157362: [RISCV] Add MC layer support for Zicfilp.

[Philip Reames via Phabricator via cfe-commits]

reames added inline comments.



Comment at: llvm/docs/RISCVUsage.rst:120
  ``Zicboz``   Assembly Support
+ ``Zicfilp``  Assembly Support
  ``Zicntr``   (`See Note <#riscv-i2p1-note>`__)

This in the wrong place; this is not a ratified extension.  

You need to link to the right spec version as well.  This is particularly 
important here as this spec has gone through a ton of churn, and may still do 
so.  



Comment at: llvm/test/MC/RISCV/zicfilp-valid.s:17
+
+# CHECK-ASM-AND-OBJ: auipc zero, 22
+# CHECK-ASM: encoding: [0x17,0x60,0x01,0x00]

If the proper extension is provided to llvm-objdump, we really shouldn't be 
disassembling this as the auipc name, it should be the landing pad mnemonic.  

I think this is just the use of -m no-aliases above.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157362

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

[PATCH] D155290: [PGO] Use Unique Profile Files when New Processes are Forked

[Qiongsi Wu via Phabricator via cfe-commits]

qiongsiwu1 added a comment.

In D155290#4582825 , @MaskRay wrote:

> Using `%p` should not magically change the behavior and I am unsure the 
> result is better for PGO. 
> If we decide to provide such a mode, it needs to be opt-in by adding a new 
> format specifier or API.
> Can you elaborate separate profile files are going to be useful?

Thanks again for taking a look at this patch!

Sure! We are generating separate profiles through `%p` for two reasons. First, 
we think that `%p` implies a profile per process, even if `exec` is not called 
after `fork`. It is unexpected that with `%p`, a program that forks child 
processes only create one profile file from the parent. This is why we did not 
create a new API or create a new pattern. Second, we are trying to catch 
non-gracefully terminated processes during profile generate. We observed a 
scenario where a hot function having all 0 counters. The reasons was the child 
process was created by a fork (but no `exec` followed), and then was terminated 
probably by a signal, instead of going through `exit()`. Such a process did not 
have a chance to write back the profile for merging. The parent process took a 
different call path and never called the hot function. We would like to detect 
such a situation for a particular process. Hence this patch creates an empty 
profile file if a process is not terminated gracefully. Using `%p` was probably 
the most natural choice when debugging such issues.

One can argue that we may look into the continuous mode to manage abnormal 
terminations during profile generate. I have not looked too closely at that, 
but we think it is still useful for a user to tell that some processes did not 
write any profile back, for debugging, or for diagnostics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155290

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

[PATCH] D156784: [AArch64][PAC] Declare FPAC subtarget feature

[Anatoly Trosinenko via Phabricator via cfe-commits]

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

Looks like adding FeatureFPAC may require changes in more places - need to 
investigate whether I have to add something like AEK_FPAC constant, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156784

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