[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; thanks!

[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 wa

[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]; // exp

[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 any

[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] 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 (!TM.getTargetTriple().isOS

[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/ h

[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)) CmdArgs

[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] 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 test

[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: > hubert.reinterpr

[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 w

[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] 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. Th

[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 co

[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] 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 offse

[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#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 two

[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] 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] 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 > tem

[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 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] 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: https://reviews.llvm.

[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 ''ゆ' (0

[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 https://revie

[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] D76696: [AST] Build recovery expressions by default for C++.

hubert.reinterpretcast added a comment. We have also encountered crashes in downstream testing caused by this using just the vanilla source from trunk. When there is a proposed fix, please let us know so we can test. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

hubert.reinterpretcast added a comment. > In D76696#1944014 , > @hubert.reinterpretcast wrote: > >> We have also encountered crashes in downstream testing caused by this using >> just the vanilla source from trunk. When there is a proposed fix, please le

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

hubert.reinterpretcast added a comment. In D76696#1944784 , @sammccall wrote: > The general scheme is probably common: unresolved expr -> ??? -> an > expression is dependent but not marked as such -> constant evaluation crashes. > > But the ??? matters, a

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1946612 , @jfb wrote: > What's the verdict then? It sounds like we are looking for `-fzero-union-padding`. That's been where the discussion has left off twice for months. Repository: rG LLVM Github M

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1946757 , @rsmith wrote: > 3. After doing (1), extend `__builtin_bit_cast` support to properly handle > padding bits. > 4. After doing (1) and (2), extend constant aggregate emission to always zero > padd

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast added inline comments. Comment at: llvm/include/llvm/MC/MCDirectives.h:47 + MCSA_WeakReference, ///< .weak_reference (MachO) + MCSA_WeakDefAutoPrivate ///< .weak_def_can_be_hidden (MachO) }; DiggerLin wrote: > @hubert.rei

[PATCH] D75056: [Driver] Default to -fno-common

hubert.reinterpretcast added a comment. I have no objections; however, it may help for the title of the patch to clearly indicate "for all targets". CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75056/new/ https://reviews.llvm.org/D75056 _

[PATCH] D75056: [Driver] Default to -fno-common for all targets

hubert.reinterpretcast added inline comments. Comment at: clang/docs/ReleaseNotes.rst:87 +- -fno-common has been enabled as the default for all targets. jyknight wrote: > Might be nice to expand upon this somewhat, to give users a clue what it > means to the

[PATCH] D75523: [www] cxx_status: Update title to mention C++20

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: rsmith, aaron.ballman. Herald added a project: clang. The document covers the Clang implementation status of the "upcoming C++20 standard". Update the title to match. Repository: rG LLVM Github Monorepo htt

[PATCH] D75524: [www] cxx_status: Update Reflection TS to Cologne draft

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: rsmith, aaron.ballman. Herald added a project: clang. As of the 2019 Cologne meeting, according to its minutes (N4826), N4818 is the draft of the Reflection TS. Repository: rG LLVM Github Monorepo https://r

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

hubert.reinterpretcast added a comment. In D69356#1726121 , @lhames wrote: > In D69356#1726074 , @beanz wrote: > > > ... It seems to me that maybe a more appropriate approach is that > > `LLVM_SUPPORT_PLUGINS` impl

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

hubert.reinterpretcast added a comment. In D69356#1726354 , @beanz wrote: > Can you please explain how the "no dead strip" semantic is harmful for > plug-in support, and how implying no dead strip for plugins is any different > from the current implement

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

hubert.reinterpretcast added a comment. In D69356#1727381 , @beanz wrote: > (1) dead stripping support is useful independent of plugins, so it is > valuable to have that option be separate Okay. >> Just because the current implementation of plug-in sup

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

hubert.reinterpretcast added a comment. In D69356#1727484 , @beanz wrote: > We should not be adding more variables that are passed around by CMake's > scope inheritance. Instead if we need to change this we should do it > correctly. Just to clarify, th

[PATCH] D69620: Add AIX assembler support

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:37 + + // Specify the mode in which the as command operates. + if (IsArch32Bit) { Refer to the other comment regarding confusion of "as" with the English word. =

[PATCH] D70675: [AIX] Disable clang python binding tests

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. Agreed. LGTM as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70675/new/ https://reviews.llvm.org/D70675 ___

[PATCH] D69620: Add AIX assembler support

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:68 + if (Inputs.size() != 1) +llvm_unreachable("Invalid number of input files."); + const InputInfo &II = Inputs[0]; `llvm_unreachable` is not the right solution i

[PATCH] D69620: Add AIX assembler support

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:70 + const InputInfo &II = Inputs[0]; + if (II.isFilename()) +CmdArgs.push_back(II.getFilename()); Should is also be checked as being either a filename or "nothing

[PATCH] D69620: Add AIX assembler support

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM with minor change request to a comment. Thanks. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:68 + if (Inputs.size() != 1) +llvm_unreachable("Invalid number of input files."); +

[PATCH] D75524: [www] cxx_status: Update Reflection TS to Cologne draft

hubert.reinterpretcast added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75524/new/ https://reviews.llvm.org/D75524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D75523: [www] cxx_status: Update title to mention C++20

hubert.reinterpretcast added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75523/new/ https://reviews.llvm.org/D75523 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D75524: [www] cxx_status: Update Reflection TS to Cologne draft

This revision was automatically updated to reflect the committed changes. Closed by commit rGdfaafbab4687: [www] cxx_status: Update Reflection TS to Cologne draft (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D755

[PATCH] D75523: [www] cxx_status: Update title to mention C++20

This revision was automatically updated to reflect the committed changes. Closed by commit rGf4076ad64075: [www] cxx_status: Update title to mention C++20 (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75523/new/

[PATCH] D75716: [clangd] Have visibleNamespaces() and getEligiblePoints() take a LangOptions rather than a FormatStyle

hubert.reinterpretcast added a comment. It looks like this is causing bot failures (http://lab.llvm.org:8011/builders/clang-ppc64le-rhel/builds/1881/steps/build%20stage%201/logs/stdio): : && /home/buildbots/clang.9.0.0/bin/clang++ --gcc-toolchain=/opt/rh/devtoolset-7/root/usr -fPIC -fPIC -fv

[PATCH] D75716: [clangd] Have visibleNamespaces() and getEligiblePoints() take a LangOptions rather than a FormatStyle

hubert.reinterpretcast added a comment. In D75716#1916032 , @hubert.reinterpretcast wrote: > It looks like this is causing bot failures > (http://lab.llvm.org:8011/builders/clang-ppc64le-rhel/builds/1881/steps/build%20stage%201/logs/stdio): Fix attempt

[PATCH] D75969: [clangd] Link libClangDaemonTweaks to libClangFormat

hubert.reinterpretcast added a comment. Thanks for the prompt response. It seems our fix attempts happened "concurrently" and we ended up with pretty much the same fix. I had committed rG48121a5743b684def33d158391c5424626de28e2

[PATCH] D75969: [clangd] Link libClangDaemonTweaks to libClangFormat

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG48121a5743b6 (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

hubert.reinterpretcast added a comment. In D69825#1916865 , @daltenty wrote: > On AIX and PPC LE Linux after this change we are seeing invalid accesses when > the backend asserts/fatal_errors. Looks like the driver and CC1 now share > some global TimerGr

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

hubert.reinterpretcast added inline comments. Comment at: llvm/include/llvm/Support/FormattedStream.h:44 + /// PartialUTF8Char - Either empty or a prefix of a UTF-8 character which + /// should be prepended to the buffer for the next call to ComputePosition.

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/Support/FormattedStream.cpp:64 +} else { + // The first few bytes from the buffer will complete the code-point. + // Concatenate them and process their effect on the line ane column

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4232 QualType Ty) const { - if (getTarget().getTriple().isOSDarwin()) { + // TODO: Add AIX ABI Info. Currently we are relying on PPC32_SVR4_ABIInfo

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/Support/FormattedStream.cpp:59 +size_t NumBytes = getNumBytesForUTF8(PartialUTF8Char[0]); +if (NumBytes > (PartialUTF8Char.size() + Size)) { + // If we still don't have enough bytes for a complete cod

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

hubert.reinterpretcast added a comment. I have no further concerns with this patch (although I would suggest editing the description to use "Unicode" capitalized). I will leave approval of the patch to one of the requested reviewers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4176 +/// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ABI information, used by PowerPC ELF +/// (SVR4), Darwin and AIX. class PPC32_SVR4_ABIInfo : public DefaultABIInfo { I w

[PATCH] D73307: Unique Names for Functions with Internal Linkage

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1135 +llvm::MD5 Md5; +Md5.update(getModule().getSourceFileName()); +llvm::MD5::MD5Result R; davidxl wrote: > rnk wrote: > > davidxl wrote: > > > Source filen

[PATCH] D73307: Unique Names for Functions with Internal Linkage

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1135 +llvm::MD5 Md5; +Md5.update(getModule().getSourceFileName()); +llvm::MD5::MD5Result R; tmsriram wrote: > hubert.reinterpretcast wrote: > > davidxl wrote

[PATCH] D72736: [AIX] Add improved interface for retrieving load module paths

hubert.reinterpretcast requested changes to this revision. hubert.reinterpretcast added inline comments. This revision now requires changes to proceed. Comment at: llvm/lib/Support/Unix/Path.inc:243 +#elif defined(_AIX) + return getLoadModuleFilenameForFunction(MainAddr)->c_str(

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

hubert.reinterpretcast added a comment. In D65761#1814968 , @ajpaverd wrote: > The CFGuard library shouldn't be needed for targets other than ARM, AArch64, > and X86, and it's only being built for these targets. However, it looks like > `llvm-build` is a

[PATCH] D41217: [Concepts] Concept Specialization Expressions

hubert.reinterpretcast added inline comments. Comment at: lib/AST/ExprCXX.cpp:1478 + { +// We do not want error diagnostics escaping here. +Sema::SFINAETrap Trap(S); saar.raz wrote: > hubert.reinterpretcast wrote: > > saar.raz wrote: > > > faisalv wrote:

[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: > > Th

[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) && +Args.hasArg(options::OPT_fno_da

[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/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 writable

[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 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, false)

[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 ``-fdata-

[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 -triple=powerpc64le-unknown-li

[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 `-fprofile

[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 rel

[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 i

[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 throug

[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`. Repository:

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

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

[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] 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 Monorep

[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 diagnostic.

[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 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 -fkeep-sta

[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 // CHECK:

[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] 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] 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] 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] 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 a

[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 the

[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 R

[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] 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 maintainers

[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 is

[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

<    2   3   4   5   6   7   8   >