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

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/static-assert.cpp:287 + static_assert((char16_t)L'ゆ' == L"C̵̭̯̠̎͌ͅť̺"[1], ""); // expected-error {{failed}} \ + // expected-note {{evaluates to ''ゆ'

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16970 + OS << '\''; + WriteCharValueForDiagnostic(CodeUnit, BTy, TyWidth, OS); + OS << "' (0x" To have the diagnostic printer handle separating

[PATCH] D153907: [AIX] [TOC] Add -mtocdata/-mno-tocdata options on AIX

hubert.reinterpretcast added inline comments. Comment at: clang/docs/UsersManual.rst:3161 +static storage duration, including static data members of classes and +block-scope static variables will be marked with the toc-data attribute. +Alternatively, the user can specify a comma

[PATCH] D158739: AIX: Issue an error when specifying an alias for a common symbol

hubert.reinterpretcast added a comment. Aside from the comments Digger has made, I have no additional concerns about this patch. It is an improvement (although there are adjacent cases that need further handling). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/static-assert.cpp:287 + static_assert((char16_t)L'ゆ' == L"C̵̭̯̠̎͌ͅť̺"[1], ""); // expected-error {{failed}} \ + // expected-note {{evaluates to ''ゆ'

[PATCH] D158158: [clang] Set FP options in Sema when instantiating CompoundStmt

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM; thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158158/new/ https://reviews.llvm.org/D158158

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16920-16926 +if (llvm::ConvertCodePointToUTF8(Value, Ptr)) { + OS << StringRef(Arr, Ptr - Arr); +} else { + OS << "\\x" + << llvm::format_hex_no_prefix(Value,

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16867-16868 +/// Convert character's code unit value to a string. +/// The code point needs to be zero-extended to 32-bits. +static void WriteCharValueForDiagnostic(uint32_t Value, const

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

hubert.reinterpretcast added a comment. Thanks, @cor3ntin, for addressing my feedback. I am not familiar enough with various aspects of it to approve it, but I see Aaron has already approved it and I think all comments have been addressed. I also appreciate that the patch helps towards making

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16876 + // other types. + if (CodePoint <= UCHAR_MAX) { +StringRef Escaped = escapeCStyle(CodePoint); cor3ntin wrote: > hubert.reinterpretcast wrote: > > For types

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

hubert.reinterpretcast added a comment. In D155610#4586213 , @abhina.sreeskantharajan wrote: > I've discussed offline with @hubert.reinterpretcast and agree with him that > with the addition of fexec-charset support, the set of characters deemed >

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Parse/ParseDecl.cpp:431-436 +if (Expr.isInvalid()) { + SawError = true; + break; +} else { + Exprs.push_back(Expr.get()); +} cor3ntin wrote: > aaron.ballman wrote: > >

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

hubert.reinterpretcast added a comment. In D155610#4575579 , @cor3ntin wrote: > @hubert.reinterpretcast It does not, Unicode characters are only escaped in > Diagnostics.cpp, and I think this is what we want. Thanks @cor3ntin for the insight. I agree

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16899 + uint32_t CodePoint = static_cast(V.getInt().getZExtValue()); + PrintCharLiteralPrefix(BTy->getKind(), OS); + OS << '\''; cor3ntin wrote:

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16899 + uint32_t CodePoint = static_cast(V.getInt().getZExtValue()); + PrintCharLiteralPrefix(BTy->getKind(), OS); + OS << '\'';

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

hubert.reinterpretcast added inline comments. Comment at: clang/test/Parser/c2x-attributes.c:29 +[[deprecated(L"abc")]] void unevaluated_string(void); +// expected-warning@-1 {{encoding prefix 'L' on an unevaluated string literal has no effec}} + "Typo" fix.

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16930-16936 +case BuiltinType::Char_S: +case BuiltinType::Char_U: +case BuiltinType::Char8: +case BuiltinType::Char16: +case BuiltinType::Char32: +

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16899 + uint32_t CodePoint = static_cast(V.getInt().getZExtValue()); + PrintCharLiteralPrefix(BTy->getKind(), OS); + OS << '\''; cor3ntin wrote:

[PATCH] D154359: [clang] Reset FP options before template instantiation

hubert.reinterpretcast added a comment. This patch seems to be direct cause of a regression: https://github.com/llvm/llvm-project/issues/64605. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154359/new/ https://reviews.llvm.org/D154359

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

hubert.reinterpretcast added a comment. In D155610#4575579 , @cor3ntin wrote: > @hubert.reinterpretcast It does not, Unicode characters are only escaped in > Diagnostics.cpp, and I think this is what we want. > Currently, llvm assume UTF-8 terminal,

[PATCH] D157201: [Clang] Support qualified name as member designator in offsetof

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/offsetof.cpp:106 +int x3[__builtin_offsetof(struct X2, X2::static_a) == 0 ? 1 : -1]; // expected-error{{no member named 'static_a'}} +int x4[__builtin_offsetof(struct X2, X2::X2) == 0 ? 1 : -1]; //

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

hubert.reinterpretcast added a comment. In D153701#4563919 , @yronglin wrote: > 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

[PATCH] D156596: [Clang] Produce a warning instead of an error in unevaluated strings before C++26

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. In D156596#4573354 , @aaron.ballman wrote: > LGTM but please wait for @hubert.reinterpretcast to confirm the new tests > cover what he was looking for. LGTM;

[PATCH] D157201: [Clang] Support qualified name as member designator in offsetof

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/offsetof.cpp:104 +struct X2 { int a; static int static_a; }; +int x2[__builtin_offsetof(struct X2, X2::a) == 0 ? 1 : -1]; +int x3[__builtin_offsetof(struct X2, X2::static_a) == 0 ? 1 : -1]; //

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

hubert.reinterpretcast added subscribers: abhina.sreeskantharajan, hubert.reinterpretcast. hubert.reinterpretcast added a comment. @abhina.sreeskantharajan, does this patch assume too much about the characters displayable for diagnostic output? CHANGES SINCE LAST ACTION

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

hubert.reinterpretcast added inline comments. Comment at: clang/test/Parser/cxx0x-attributes.cpp:450-451 +namespace P2361 { +[[deprecated(L"abc")]] void a(); // expected-error{{an unevaluated string literal cannot have an encoding prefix}} +[[nodiscard("\123")]] int b(); //

[PATCH] D151567: [LLVM][Support] Report EISDIR when opening a directory on AIX

hubert.reinterpretcast added a comment. I believe the unit tests should be updated to ensure that we get `EISDIR` when opening directories for reading and for writing: `llvm/unittests/Support/Path.cpp`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D151567: [LLVM][Support] Report EISDIR when opening a directory on AIX

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/Support/Unix/Path.inc:1020 +struct stat Status; +if (stat(P.begin(), ) == -1) + return std::error_code(errno, std::generic_category()); Please try using `fstat` on the result of

[PATCH] D156596: [Clang] Produce a warning instead of an error in unevaluated strings before C++26

hubert.reinterpretcast added a comment. In D156596#4558097 , @cor3ntin wrote: > Note: we are waiting for feedback from @hubert.reinterpretcast's people > before progressing this. The "full" build hit some (unrelated) snags, but the initial results look

[PATCH] D156596: [Clang] Produce a warning instead of an error in unevaluated strings before C++26

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:290 + "encoding prefix '%0' on an unevaluated string literal has no effect" + "%select{| and is incompatible with c++2c}1">, + InGroup>; aaron.ballman

[PATCH] D156596: [Clang] Produce a warning instead of an error in unevaluated strings before C++26

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:290 + "encoding prefix '%0' on an unevaluated string literal has no effect" + "%select{| and is incompatible with c++2c}1">, + InGroup>; I am not seeing

[PATCH] D156596: [Clang] Produce a warning instead of an error in unevaluated strings before C++26

hubert.reinterpretcast added a comment. In D156596#4546742 , @aaron.ballman wrote: > If so, I'd like @hubert.reinterpretcast to mention if this works for his > needs. We are putting together a build for the (known) affected groups. I will report back

[PATCH] D156596: [Clang] Produce a warning instead of an error in unevaluated strings before C++26

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/static-assert.cpp:44-45 +static_assert(false, L"\x1ff"// expected-warning {{encoding prefix 'L' on an unevaluated string literal has no effect and is incompatible with c++2c}} \ +

[PATCH] D105759: Implement P2361 Unevaluated string literals

hubert.reinterpretcast added a comment. In D105759#4543246 , @aaron.ballman wrote: > I'd recommend we change the diagnostic to be a warning that defaults to an > error so that users who are caught by the changes can still disable the > diagnostic

[PATCH] D105759: Implement P2361 Unevaluated string literals

hubert.reinterpretcast added a comment. In D105759#4541813 , @cor3ntin wrote: > In D105759#4540716 , > @hubert.reinterpretcast wrote: > >>> I hope this patch may allow to gather some data on that. >> >>

[PATCH] D105759: Implement P2361 Unevaluated string literals

hubert.reinterpretcast added a comment. > I hope this patch may allow to gather some data on that. @cor3ntin, I have reports that applications having encoding prefixes in `static_assert` are failing to build. The committee did not adopt the subject paper as a "DR resolution". Is it possible to

[PATCH] D155544: [AIX][TLS][clang] Add -maix-small-local-exec-tls clang option.

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM for landing after D156203 and D155600 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D155501: Add new option -fkeep-persistent-storage-variables to Clang release notes

This revision was automatically updated to reflect the committed changes. Closed by commit rGcead1497ae0c: Add new option -fkeep-persistent-storage-variables to Clang release notes (authored by qianzhen, committed by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D155544: [AIX][TLS] Add -maix-small-local-exec-tls option.

hubert.reinterpretcast added a comment. In D155544#4523857 , @amyk wrote: > Wouldn't this patch need to land before the back-end patch, because I > introduce the option here, and then I use it in the backend patch? Please move the `llvm/` changes into

[PATCH] D155544: [AIX][TLS] Add -maix-small-local-exec-tls option.

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Driver/Options.td:4095 + HelpText<"Produce a faster access sequence for local-exec TLS variables " + "where the offset from the thread pointer value is encoded as an " + "immediate

[PATCH] D155544: [AIX][TLS] Add -maix-small-local-exec-tls option.

hubert.reinterpretcast added a comment. Patch should not land before back-end patch. I also suggest having the patch incorporate the new option into the Clang release notes before it lands. Comment at: llvm/lib/Target/PowerPC/PPC.td:324 + "Produce a faster

[PATCH] D155501: Add new option -fkeep-persistent-storage-variables to Clang release notes

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM; thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155501/new/ https://reviews.llvm.org/D155501

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

hubert.reinterpretcast added a comment. In D153536#4512897 , @dblaikie wrote: > but at least at a first blush I can't reproduce the failures shown... @dblaikie, you //did// reproduce the issue with the members. Both entries have DW_AT_decl_line 2 and

[PATCH] D139586: [Clang][C++23] Lifetime extension in range-based for loops

hubert.reinterpretcast added a comment. In D139586#4363725 , @cor3ntin wrote: > @hubert.reinterpretcast I'll try to look at that but unless I'm mistaken, > the wording excludes function parameters > >> The fourth context is when a temporary object

[PATCH] D155501: Add new option -fkeep-persistent-storage-variables to Clang release notes

hubert.reinterpretcast added inline comments. Comment at: clang/docs/ReleaseNotes.rst:259-260 +- ``-fkeep-persistent-storage-variables`` has been implemented to keep all + variables that have a persistent storage duration, including global, static + and thread-local variables,

[PATCH] D150221: Add option -fkeep-persistent-storage-variables to emit all variables that have a persistent storage duration

This revision was automatically updated to reflect the committed changes. Closed by commit rGbb6ab91b1dcd: Add option -fkeep-persistent-storage-variables to emit all variables that have… (authored by qianzhen, committed by hubert.reinterpretcast). Changed prior to commit:

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

hubert.reinterpretcast added inline comments. Comment at: clang/docs/ReleaseNotes.rst:136-139 +- Implemented `P2169R4: A nice placeholder with no name `_. This allows using `_` + as a variable name multiple times in the same scope and is supported in

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:8901-8914 + // [P2718R0] Lifetime extension in range-based for loops. + // + // 6.7.7 [class.temporary] p5: + // There are four contexts in which temporaries are destroyed at a different

[PATCH] D150221: Add option -fkeep-persistent-storage-variables to emit all variables that have a persistent storage duration

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. > Furthermore, there is some discussion over covering more than just variables, > but also artifacts with reasonable mangled names such as the symbols for >

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:8901-8914 + // [P2718R0] Lifetime extension in range-based for loops. + // + // 6.7.7 [class.temporary] p5: + // There are four contexts in which temporaries are destroyed at a different

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:8901-8914 + // [P2718R0] Lifetime extension in range-based for loops. + // + // 6.7.7 [class.temporary] p5: + // There are four contexts in which temporaries are destroyed at a different

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:8901-8914 + // [P2718R0] Lifetime extension in range-based for loops. + // + // 6.7.7 [class.temporary] p5: + // There are four contexts in which temporaries are destroyed at a different

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

hubert.reinterpretcast added a comment. In D153536#4483191 , @hubert.reinterpretcast wrote: > Similarly, only one local variable out of two in the same line reported: I can confirm that adding a lexical block scope causes both variables to be recorded

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

hubert.reinterpretcast added a comment. In D153536#4483182 , @hubert.reinterpretcast wrote: > If `llvm-dwarfdump` is to be believed, it looks like a compiler bug (two > members reported at the same offset). Similarly, only one local variable out of

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:46 +int arr[2] = {0, 1}; +static auto [_, _] = arr; // expected-error {{redefinition of '_'}} \ +// expected-note {{previous definition is

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

hubert.reinterpretcast added a comment. In D153536#4479275 , @hubert.reinterpretcast wrote: > It seems the class member case trips up debuggers. If `llvm-dwarfdump` is to be believed, it looks like a compiler bug (two members reported at the same

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

hubert.reinterpretcast added a comment. In D153536#4474066 , @cor3ntin wrote: > Seems to work well enough @hubert.reinterpretcast It seems the class member case trips up debuggers. union U { struct A { int _, _; } a; struct B { int x, y; } b;

[PATCH] D150221: Add option -fkeep-persistent-storage-variables to emit all variables that have a persistent storage duration

hubert.reinterpretcast added a comment. Update (looping back from offline discussion): The LTO use case is covered. There was some confusion over which meaning of "static" was meant by `-fkeep-static-consts`. The "static" meant was storage duration. Furthermore, there is some discussion over

[PATCH] D153600: Implement -frecord-command-line for XCOFF

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:2974 +RSOS << "@(#)" << MDS->getString(); +RSOS.write('\0'); + } hubert.reinterpretcast wrote: > stephenpeckham wrote: > > I would use a newline here.

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2 +// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused %s + +void static_var() { hubert.reinterpretcast wrote: > cor3ntin wrote:

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2 +// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused %s + +void static_var() { cor3ntin wrote: > cor3ntin wrote: > > cor3ntin

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2 +// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused %s + +void static_var() { hubert.reinterpretcast wrote: >

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2 +// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused %s + +void static_var() { hubert.reinterpretcast wrote: > Can we have

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2 +// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused %s + +void static_var() { Can we have tests for: ``` struct { int _, _; }

[PATCH] D152021: [clang][AIX] Fix Overly Strict LTO Option Checking against `data-sections` when `mxcoff-roptr` is in Effect

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM with minor comment. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:732-735 UseSeparateSections))

[PATCH] D143670: Stop claiming we support [[carries_dependency]]

hubert.reinterpretcast added a comment. Updated patch limited to changing the feature test macro value would match Varna meeting outcome (changes to allow macro to be `0` accepted as DR). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143670/new/

[PATCH] D153600: Implement -frecord-command-line for XCOFF

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2338 // Emit bytes for llvm.commandline metadata. - emitModuleCommandLines(M); + // The command line metadata waas emitted earlier on XCOFF. + if

[PATCH] D150221: Add option -fkeep-static-variables to emit all static variables

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.def:475 +/// Whether to emit all variables that have a persisent storage duration, +/// including global, static and thread local variables. Minor nit: Typo.

[PATCH] D151567: [LLVM][Support] Report EISDIR when opening a directory on AIX

hubert.reinterpretcast added a comment. In D151567#4458232 , @hubert.reinterpretcast wrote: > @azhan92, please incorporate a revert of > https://reviews.llvm.org/rG64ca650cf9f180cc0b68c0005639028084066e10. Since it > is an `XFAIL`. once the problem is

[PATCH] D151567: [LLVM][Support] Report EISDIR when opening a directory on AIX

hubert.reinterpretcast added a comment. @azhan92, rG6ace52e5e49cff6664fc301fa4985fc28c88f26f and rGc14df228ff3ca73e3c5c00c495216bba56665fd5 should also

[PATCH] D151567: [LLVM][Support] Report EISDIR when opening a directory on AIX

hubert.reinterpretcast added a comment. @azhan92, please incorporate a revert of https://reviews.llvm.org/rG64ca650cf9f180cc0b68c0005639028084066e10. Since it is an `XFAIL`. once the problem is fixed, the test will end up being an "unexpected success" unless we remove the `XFAIL`.

[PATCH] D150221: Add option -fkeep-static-variables to emit all static variables

hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/keep-static-variables.cpp:1 +// RUN: %clang_cc1 -fkeep-static-variables -emit-llvm %s -o - -triple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefixes=CHECK,CHECK-ELF +// RUN: %clang_cc1

[PATCH] D150221: Add option -fkeep-static-variables to emit all static variables

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2201-2210 + if ((CodeGenOpts.KeepStaticConsts || CodeGenOpts.KeepStaticVariables) && D && + isa(D)) { const auto *VD = cast(D); -if (VD->getType().isConstQualified() && -

[PATCH] D150221: Add option -fkeep-static-variables to emit all static variables

hubert.reinterpretcast added a comment. In D150221#4351249 , @rjmccall wrote: > Force-emitting every `static` in the translation unit can be very expensive; > there are a lot of headers that declare all their constants as `static > const`. And

[PATCH] D150221: Add option -fkeep-static-variables to emit all static variables

hubert.reinterpretcast added a comment. In D150221#4350761 , @rjmccall wrote: > I'm not sure if it's better to represent that by using > `__attribute__((used))` on every global variable or by setting something more > globally in the module. The latter

[PATCH] D150221: Add option -fkeep-static-variables to emit all static variables

hubert.reinterpretcast added a comment. In D150221#4347840 , @efriedma wrote: >> This is an adaptation of the IBM XL compiler's -qstatsym option, which is >> meant to generate symbol table entries for static variables. An artifact of >> that compiler

[PATCH] D150221: Add option -fkeep-static-variables to emit all static variables

hubert.reinterpretcast added a comment. In D150221#4343546 , @efriedma wrote: > It's not unprecedented to add flags to copy the behavior of other compilers, > to make porting easier, especially when it doesn't place much burden on > compiler

[PATCH] D150597: [clang][AIX] Adding Revised xcoff-roptr CodeGen Test Case

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM; thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150597/new/ https://reviews.llvm.org/D150597

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM with minor comments! Comment at: clang/lib/Driver/ToolChains/AIX.cpp:132 +// possible. Then `-bforceimprw` changes such sections to

[PATCH] D150221: Add option -fkeep-static-variables to emit all static variables

hubert.reinterpretcast added a comment. In D150221#4332142 , @erichkeane wrote: >> This is intended to prevent "excessive transformation" to enable migration >> of existing applications (using a non-Clang compiler) where users further >> manipulate

[PATCH] D150221: Add option -fkeep-static-variables to emit all static variables

hubert.reinterpretcast added a comment. In D150221#4330504 , @MaskRay wrote: > Can you give a more compelling reason that this option is needed? This is intended to prevent "excessive transformation" to enable migration of existing applications (using

[PATCH] D146399: [AIX][Clang][K] Create `-K` Option for AIX.

hubert.reinterpretcast added a comment. In D146399#4244479 , @francii wrote: > Do we need a test case for the diagnostic? If we are only supporting one > target, which triple would be used for the test? I think we do need a test case for the

[PATCH] D146399: [AIX][Clang][K] Create `-K` Option for AIX.

hubert.reinterpretcast added a comment. @francii, what happens when `-K` is used on a pure-compile (`-c`) invocation? Do we get an "unused" message? Should we be testing that? I think we should be testing the diagnostic for the "wrong target" usage as well? Repository: rG LLVM Github

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:125 + // The `-mroptr` option places constants in RO sections as much as possible. + // Then `-bforceimprw` changes such sections to RW if they contain imported Old

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

hubert.reinterpretcast added inline comments. Comment at: clang/docs/ReleaseNotes.rst:317 + ``-fno-data-sections``. When ``-mxcoff-roptr`` is in effect at link time, + read-only data sections with relocatable address values that resolve to + imported symbols are made

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:647 +def err_roptr_requires_data_sections: Error<"-mxcoff-roptr is supported only with -fdata-sections">; +def err_roptr_cannot_build_shared: Error<"-mxcoff-roptr is not

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:714-716 +if (!Args.hasFlag(options::OPT_fdata_sections, + options::OPT_fno_data_sections, UseSeparateSections) && +

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5250 + if (Args.hasArg(options::OPT_mroptr) || Args.hasArg(options::OPT_mno_roptr)) { +bool HasRoptr = qiongsiwu1 wrote: > hubert.reinterpretcast wrote: > >

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

hubert.reinterpretcast added inline comments. Comment at: clang/test/Driver/ppc-roptr.c:10 +// ROPTR-NOT: "-mroptr" +// ROPTR-NOT: "-bforceimprw" + hubert.reinterpretcast wrote: > w2yehia wrote: > > hubert.reinterpretcast wrote: > > > Do we pass the option

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5250 + if (Args.hasArg(options::OPT_mroptr) || Args.hasArg(options::OPT_mno_roptr)) { +bool HasRoptr = This only checks for `-m[no-]roptr` when the front-end

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

hubert.reinterpretcast added inline comments. Comment at: clang/docs/ReleaseNotes.rst:231 + address values in the read-only data section. This option is intended to + be used with the ``-fdata-sections`` option. When ``-mroptr`` is in effect, + read-only data sections with

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

hubert.reinterpretcast added inline comments. Comment at: clang/test/Driver/ppc-roptr.c:10 +// ROPTR-NOT: "-mroptr" +// ROPTR-NOT: "-bforceimprw" + w2yehia wrote: > hubert.reinterpretcast wrote: > > Do we pass the option through to the LTO codegen when

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

hubert.reinterpretcast added a subscriber: w2yehia. hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/PowerPC/aix-roptr.c:9-10 +// RUN: -S <%s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR +// RUN: not %clang_cc1

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

hubert.reinterpretcast added inline comments. Comment at: clang/docs/ReleaseNotes.rst:229-231 +- Introduced the ``-mroptr`` option to place constant objects with relocatable + address values in the ready-only data section. This option is intended to + be used with the

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

hubert.reinterpretcast requested changes to this revision. hubert.reinterpretcast added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5251 + if (Args.hasFlag(options::OPT_mroptr, options::OPT_mno_roptr,

[PATCH] D143670: Stop claiming we support [[carries_dependency]]

hubert.reinterpretcast added inline comments. Comment at: clang/test/Preprocessor/has_attribute.cpp:51 // FIXME(201806L) CHECK: assert: 0 -// CHECK: carries_dependency: 200809L +// CHECK: carries_dependency: 0 // CHECK: deprecated: 201309L aaron.ballman wrote:

[PATCH] D142867: [Clang] Add machinery to catch overflow in unary minus outside of a constant expression context

hubert.reinterpretcast added a comment. In D142867#4108079 , @eaeltsin wrote: > The warning now fires even if overflow is prevented with `if constexpr`: > > if constexpr (width <= 64) { > if constexpr (width == 64) { > return 1; > } >

[PATCH] D143891: [Clang] Adjust triviality computation in QualType::isTrivialType to C++20 cases.

hubert.reinterpretcast added a comment. In D143891#4122660 , @aaron.ballman wrote: > This is an ABI breaking change, isn't it? (The type trait now returns > something different than it did before, which could change instantiations or > object layout.)

[PATCH] D143670: Stop claiming we support [[carries_dependency]]

hubert.reinterpretcast added a comment. I think it may be an option to use the `gnu++*` modes to do deliberately non-conforming things, but I believe there should be an RFC for that first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143670/new/

[PATCH] D143670: Stop claiming we support [[carries_dependency]]

hubert.reinterpretcast requested changes to this revision. hubert.reinterpretcast added inline comments. Comment at: clang/test/Preprocessor/has_attribute.cpp:51 // FIXME(201806L) CHECK: assert: 0 -// CHECK: carries_dependency: 200809L +// CHECK: carries_dependency: 0 //

  1   2   3   4   5   6   7   8   >