[PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

serge-sans-paille added inline comments. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:470 const char *const *Prefixes[DriverID::LastOption] = {nullptr}; -#define PREFIX(NAME, VALUE) static const char *const NAME[] = VALUE; +#define PREFIX(NAME, VALUE) static cons

[PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

serge-sans-paille updated this revision to Diff 482825. serge-sans-paille added a comment. Make `Prefixes` an array of StringRef too CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139881/new/ https://reviews.llvm.org/D139881 Files: clang-tools-extra/clangd/CompileCommands.cpp clang/

[PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

serge-sans-paille marked 4 inline comments as done. serge-sans-paille added a comment. New version passes `ninja check` with clang, clang-tools-extra and lld. Comment at: clang/lib/StaticAnalyzer/Core/CallDescription.cpp:39 ento::CallDescription::CallDescription(CallDescriptio

[PATCH] D32550: Supress all uses of LLVM_END_WITH_NULL

serge-sans-paille created this revision. Variadic templates are type safer, easier to use and don't require the extra sentinel. https://reviews.llvm.org/D32550 Files: lib/CodeGen/CGBlocks.cpp lib/CodeGen/CGBuiltin.cpp lib/CodeGen/CGCUDANV.cpp lib/CodeGen/CGCleanup.cpp lib/CodeGen/CGE

[PATCH] D32550: Supress all uses of LLVM_END_WITH_NULL

serge-sans-paille added a subscriber: mehdi_amini. serge-sans-paille added a comment. @mehdi_amini are you okay with my previous explanation? https://reviews.llvm.org/D32550 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

[PATCH] D118169: [NFC][CodeGen] Use llvm::DenseMap for DeferredDecls

serge-sans-paille added a comment. The change looks fine to me. I've started a set of benchmark to make sure it provides the expected speedup. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118169/new/ https://reviews.llvm.org/D118169

[PATCH] D118169: [NFC][CodeGen] Use llvm::DenseMap for DeferredDecls

serge-sans-paille accepted this revision. serge-sans-paille added a comment. No performance regression on my side either (but only minor gains). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118169/new/ https://reviews.llvm.org/D118169 ___

[PATCH] D118385: [NFC] Optimize FoldingSet usage where it matters

serge-sans-paille added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:214 mutable llvm::FoldingSet ComplexTypes; - mutable llvm::FoldingSet PointerTypes; + mutable llvm::FoldingSet PointerTypes{9}; mutable llvm::FoldingSet AdjustedTypes; --

[PATCH] D118385: [NFC] Optimize FoldingSet usage where it matters

serge-sans-paille added inline comments. Comment at: llvm/include/llvm/ADT/FoldingSet.h:328 /// Add* - Add various data types to Bit data. - void AddPointer(const void *Ptr); - void AddInteger(signed I); - void AddInteger(unsigned I); - void AddInteger(long I); - void Add

[PATCH] D118532: Fix -Wreserved-identifier in presence of system macro

serge-sans-paille created this revision. serge-sans-paille added reviewers: lebedev.ri, aaron.ballman, rsmith. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Do not warn on reserved identifiers resulting from expansion of

[PATCH] D118532: Fix -Wreserved-identifier in presence of system macro

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb8290ffa9fd9: Fix -Wreserved-identifier in presence of system macro (authored by serge-sans-paille). Changed prior to commit: https://reviews.llvm

[PATCH] D118652: Cleanup header dependencies in LLVMCore

serge-sans-paille created this revision. serge-sans-paille added reviewers: RKSimon, lenary, mehdi_amini, MaskRay. Herald added subscribers: jeroen.dobbelaere, ormris, foad, dexonsmith, wenlei, jdoerfert, kerbowa, pengfei, arphaman, steven_wu, hiraditya, nhaehnle, jvesely, arsenm. Herald added a

[PATCH] D118652: Cleanup header dependencies in LLVMCore

serge-sans-paille added a comment. Hi and thanks for taking the time to go through the review. In D118652#3285601 , @MaskRay wrote: > Thanks for working on this. I spot checked some files. In general it looks > great. > > One file replaces a forward dec

[PATCH] D118652: Cleanup header dependencies in LLVMCore

serge-sans-paille added a comment. In D118652#3286018 , @mehdi_amini wrote: > I'm not sure how it'd help clients of the released version of LLVM to delay, > the development branch and the release seems fairly disconnected to me from > this point of vie

[PATCH] D118652: Cleanup header dependencies in LLVMCore

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe188aae406f3: Cleanup header dependencies in LLVMCore (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D118608: [NFC] Increase initial size of FoldingSets used in ASTContext and CodeGenTypes

serge-sans-paille added a comment. Thanks, that's helpful. Any hint about why this particular values? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118608/new/ https://reviews.llvm.org/D118608 ___ cfe-co

[PATCH] D118781: Reduce dependencies on llvm/BinaryFormat/Dwarf.h

serge-sans-paille created this revision. serge-sans-paille added reviewers: RKSimon, MaskRay, mehdi_amini, lenary. Herald added subscribers: awarzynski, sdasgup3, wenzhicui, wrengr, Chia-hungDuan, foad, dcaballe, cota, teijeong, frasercrmck, dexonsmith, rdzhabarov, tatianashp, msifontes, jurahul,

[PATCH] D112916: Confusable identifiers detection

serge-sans-paille added a comment. @rsmith : once the licensing issue has been fixed, can we merge that patch or do you have any other thought? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112916/new/ https://reviews.llvm.org/D112916 ___ cf

[PATCH] D60380: Also document -arch as -arch is mac specific

serge-sans-paille accepted this revision. serge-sans-paille added a comment. This revision is now accepted and ready to land. Thanks o/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60380/new/ https://reviews.llvm.org/D60380 __

[PATCH] D120205: Restore documentation for __builtin_assume

serge-sans-paille created this revision. serge-sans-paille added reviewers: aaron.ballman, vdelvecc. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This got removed by 6cacd420a1d72bca7809e6b516fb1e18ac6056c8

[PATCH] D120205: Restore documentation for __builtin_assume

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG79c9072dc009: Restore documentation for __builtin_assume (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D120333: [NFC][Lexer] Make access to LangOpts more consistent

serge-sans-paille added a comment. I don't object to the change but it may me wonder if there would be a way to remove the apparent redundancy in the API here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120333/new/ https://reviews.llvm.org/D120

[PATCH] D120334: [NFC][Lexer] Use more appropriate LangOptionsBase type for Lexer::LangOpts

serge-sans-paille added a comment. A small local test hints that using a ``SmallVector`` instead of a ``std::vector`` also fixes the assembly bloat. I don't know if it's worth considering as an alternative or as an extra step. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D120334: [NFC][Lexer] Use more appropriate LangOptionsBase type for Lexer::LangOpts

serge-sans-paille added a comment. https://llvm-compile-time-tracker.com/compare.php?from=79c9072dc009693477242bc1347a2a6c3e419423&to=702bdad5179ebb90c8d8a16c800ffd1b3c44744d&stat=instructions << indeed a lot of green there! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D112916: Confusable identifiers detection

serge-sans-paille updated this revision to Diff 410776. serge-sans-paille added a comment. Rebased on `main` branch and (should) fix @aaron.ballman portability issue. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112916/new/ https://reviews.llvm.org/D112916 Files: clang-tools-extra/c

[PATCH] D120404: [NFC][Lexer] Remove getLangOpts function from Lexer

serge-sans-paille accepted this revision. serge-sans-paille added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Lex/Lexer.h:313 + /// getSpelling - This is non-staic variant of getSpelling method which + /// doesn't require

[PATCH] D120484: More explicit message when failing to find a mandatory cfi ressource file

serge-sans-paille created this revision. serge-sans-paille added a reviewer: thakis. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Compiler-rt doesn't provide support file for cfi on s390x ad ppc64le (at least). When tr

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille updated this revision to Diff 341853. serge-sans-paille marked 10 inline comments as done. serge-sans-paille added a comment. Take into account latest reviews CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93095/new/ https://reviews.llvm.org/D93095 Files: clang/inclu

[PATCH] D93095: Introduce -Wreserved-identifier

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb83b23275b74: Introduce -Wreserved-identifier (authored by serge-sans-paille). Herald added a project: clang. Changed prior to commit: https://rev

[PATCH] D102164: [NFC] Syndicate reserved identifier code between macro and variables / symbols

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG91a919e8994a: [NFC] Synchronize reserved identifier code between macro and variables / symbols (authored by serge-sans-paille). Herald added a projec

[PATCH] D116110: Introduce the AttributeMask class

serge-sans-paille updated this revision to Diff 397225. serge-sans-paille added a comment. rebased on main branch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116110/new/ https://reviews.llvm.org/D116110 Files: clang/lib/CodeGen/CodeGenModule.cpp llvm/include/llvm/IR/Argument.h

[PATCH] D116110: Introduce the AttributeMask class

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9290ccc3c1a1: Introduce the AttributeMask class (authored by serge-sans-paille). Changed prior to commit: https://reviews.llvm.org/D116110?vs=3972

[PATCH] D116599: Simplify AttrBuilder storage for target dependent attributes

serge-sans-paille created this revision. serge-sans-paille added reviewers: nikic, rnk, dexonsmith. Herald added subscribers: sdasgup3, wenzhicui, wrengr, Chia-hungDuan, ormris, dcaballe, cota, teijeong, rdzhabarov, tatianashp, jdoerfert, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb,

[PATCH] D116599: Simplify AttrBuilder storage for target dependent attributes

serge-sans-paille updated this revision to Diff 397298. serge-sans-paille added a comment. upload correct patch version CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116599/new/ https://reviews.llvm.org/D116599 Files: clang/lib/CodeGen/CGAtomic.cpp clang/lib/CodeGen/CGBuiltin.cpp

[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

serge-sans-paille abandoned this revision. serge-sans-paille added a comment. Obsoleted by https://reviews.llvm.org/D116599 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115798/new/ https://reviews.llvm.org/D115798 ___ cfe-commits mailing lis

[PATCH] D116598: Document __builtin_trap and __builtin_debugtrap

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG491984c4e60c: Document __builtin_trap and __builtin_debugtrap (authored by serge-sans-paille). Herald added a project: clang. Herald added a subscrib

[PATCH] D116599: Simplify AttrBuilder storage for target dependent attributes

serge-sans-paille updated this revision to Diff 397486. serge-sans-paille added a comment. Take advice into account CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116599/new/ https://reviews.llvm.org/D116599 Files: clang/lib/CodeGen/CGAtomic.cpp clang/lib/CodeGen/CGBuiltin.cpp cla

[PATCH] D112913: Misleading bidirectional detection

serge-sans-paille updated this revision to Diff 397597. serge-sans-paille added a comment. Update bidi algorithm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112913/new/ https://reviews.llvm.org/D112913 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt clang-tools-extra/clan

[PATCH] D112913: Misleading bidirectional detection

serge-sans-paille added a comment. @upsuper I've not added extra test cases yet, but does it looks it's heading in the right direction? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112913/new/ https://reviews.llvm.org/D112913 ___ cfe-commit

[PATCH] D112913: Misleading bidirectional detection

serge-sans-paille updated this revision to Diff 397813. serge-sans-paille added a comment. Fix some parts of the bidi algorithm, and add extra test cases CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112913/new/ https://reviews.llvm.org/D112913 Files: clang-tools-extra/clang-tidy/mis

[PATCH] D116599: Simplify AttrBuilder storage for target dependent attributes

serge-sans-paille updated this revision to Diff 397831. serge-sans-paille added a comment. Minor updates CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116599/new/ https://reviews.llvm.org/D116599 Files: clang/lib/CodeGen/CGAtomic.cpp clang/lib/CodeGen/CGBuiltin.cpp clang/lib/Code

[PATCH] D112913: Misleading bidirectional detection

serge-sans-paille updated this revision to Diff 397885. serge-sans-paille added a comment. rebased on main branch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112913/new/ https://reviews.llvm.org/D112913 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt clang-tools-extra/cla

[PATCH] D112913: Misleading bidirectional detection

serge-sans-paille added a comment. Thanks @upsuper! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112913/new/ https://reviews.llvm.org/D112913 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D112913: Misleading bidirectional detection

serge-sans-paille added a comment. @aaron.ballman unfortunately I don't know any of those. If I recall correctly we found no software in the RedHat collection actually using those control characters :-/ My understanding is that we are inline with the document you mention, except the fact that 1

[PATCH] D112913: Misleading bidirectional detection

serge-sans-paille added a comment. Note that if there's a consensus on it, I can implement LRM character support. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112913/new/ https://reviews.llvm.org/D112913 ___ cfe-commits mailing list cfe-comm

[PATCH] D112913: Misleading bidirectional detection

serge-sans-paille updated this revision to Diff 398195. serge-sans-paille added a comment. rebased CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112913/new/ https://reviews.llvm.org/D112913 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt clang-tools-extra/clang-tidy/misc/Mi

[PATCH] D116599: Simplify AttrBuilder storage for target dependent attributes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd2cc6c2d0c2f: Use a sorted array instead of a map to store AttrBuilder string attributes (authored by serge-sans-paille). Herald added a subscriber:

[PATCH] D112913: Misleading bidirectional detection

serge-sans-paille added a comment. @MaskRay any blocker on that new version now that it recieved a green light from @upsuper? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112913/new/ https://reviews.llvm.org/D112913 ___ cfe-commits mailing

[PATCH] D112913: Misleading bidirectional detection

serge-sans-paille added a comment. In D112913#3233699 , @upsuper wrote: > I'd like to clarify that what I think is correct now is the algorithm to > detect unclosed explicit formatting scopes in a given string. Thanks for confirming. This check only det

[PATCH] D112913: Misleading bidirectional detection

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG35cca45b09b8: Misleading bidirectional detection (authored by serge-sans-paille). Changed prior to commit: https://reviews.llvm.org/D112913?vs=398

[PATCH] D117296: Document several clang-supported builtins

serge-sans-paille created this revision. serge-sans-paille added reviewers: aaron.ballman, rsmith. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Namely __builtin_alloca __builtin_alloca_with_align __builtin_call_with_st

[PATCH] D117296: Document several clang-supported builtins

serge-sans-paille updated this revision to Diff 400080. serge-sans-paille marked 8 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117296/new/ https://reviews.llvm.org/D117296 Files: clang/docs/LanguageExtensions.rst Index: clang/docs/LanguageExtensions.rst

[PATCH] D117296: Document several clang-supported builtins

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6cacd420a1d7: Document several clang-supported builtins (authored by serge-sans-paille). Changed prior to commit: https://reviews.llvm.org/D117296

[PATCH] D114394: Compile-time computation of string attribute hashes

serge-sans-paille added a comment. > Frontends not written in C++ will always be going through the > AttributeKey::get() API, which will be slower than the initial state (I think > -- we still have to calculate the hash dynamically, but now we also need to > intern an AttributeKey by performing

[PATCH] D114394: Compile-time computation of string attribute hashes

serge-sans-paille added a comment. *comment removed, I've been doing more detailed benchmark that imply a rework of the patch* CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114394/new/ https://reviews.llvm.org/D114394 ___ cfe-commits mailing

[PATCH] D114394: Compile-time computation of string attribute hashes

serge-sans-paille updated this revision to Diff 393058. serge-sans-paille added a comment. Use a DenseSet instead of a DenseMap to store the StringPool. Some benchmark feedback (using a release build compiled with LTO), in number of instructions | | sqlite3.c -c -O0 | sqlite3.c -S

[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

serge-sans-paille created this revision. serge-sans-paille added reviewers: nikic, dblaikie, rnk. Herald added subscribers: dexonsmith, jdoerfert, mgrang, hiraditya. serge-sans-paille requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-

[PATCH] D114394: Compile-time computation of string attribute hashes

serge-sans-paille added a comment. Update: the strategy used by ``AttrBuilder`` might not be the best way to build new attributes. I've proposed an alternative implementation in https://reviews.llvm.org/D115798. This is somehow orthogonal to that patch, but it removes the need for temporary cop

[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

serge-sans-paille added a comment. My plan mid-term plan would be to rename `AttrBuilder` into `AttrQuery` at some point, and use `SmallAttrBuilder` as the actual `AttrBuilder` in most places. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115798/ne

[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

serge-sans-paille updated this revision to Diff 394542. serge-sans-paille added a comment. Leverage the fact that AttributeSet nodes are already sorted CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115798/new/ https://reviews.llvm.org/D115798 Files: clang/include/clang/CodeGen/CodeGe

[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

serge-sans-paille added a comment. Actual benchmarks: https://llvm-compile-time-tracker.com/compare.php?from=7d97678df7f514c14b7611447dad02e9cc5168c9&to=f39a39e09e8f4f3b7dc94e4d23d9acfbf36ab2e5&stat=instructions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115798/new/ https://reviews.

[PATCH] D115798: Provide SmallAttrBuilder as a lightweight alternative to AttrBuilder

serge-sans-paille added inline comments. Comment at: llvm/include/llvm/IR/Attributes.h:974 + SmallVector EnumAttrs; + SmallVector StringAttrs; + using iterator = typename SmallVector::iterator; nikic wrote: > Just wondering if storing both in one vector would

[PATCH] D116110: Introduce the AttributeMask class

serge-sans-paille created this revision. serge-sans-paille added reviewers: nikic, rnk. Herald added subscribers: ormris, dexonsmith, jdoerfert, hiraditya. serge-sans-paille requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits.

[PATCH] D116110: Introduce the AttributeMask class

serge-sans-paille updated this revision to Diff 395677. serge-sans-paille added a comment. Minor nits CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116110/new/ https://reviews.llvm.org/D116110 Files: clang/lib/CodeGen/CodeGenModule.cpp llvm/include/llvm/IR/Argument.h llvm/include

[PATCH] D116110: Introduce the AttributeMask class

serge-sans-paille updated this revision to Diff 395692. serge-sans-paille added a comment. Herald added subscribers: foad, kerbowa, nhaehnle, jvesely, arsenm. remvove useless assert, update comment, fix build CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116110/new/ https://reviews.llvm

[PATCH] D116110: Introduce the AttributeMask class

serge-sans-paille updated this revision to Diff 395694. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116110/new/ https://reviews.llvm.org/D116110 Files: clang/lib/CodeGen/CodeGenModule.cpp llvm/include/llvm/IR/Argument.h llvm/include/llvm/IR/Attributes.h llvm/include/llvm/IR/Fun

[PATCH] D116110: Introduce the AttributeMask class

serge-sans-paille marked 2 inline comments as done. serge-sans-paille added inline comments. Comment at: llvm/include/llvm/IR/Attributes.h:993 + + bool empty() const { return Attrs.none(); } + bool hasAttributes() const { return TargetDepAttrs.empty() && Attrs.none(); } ---

[PATCH] D116110: Introduce the AttributeMask class

serge-sans-paille updated this revision to Diff 395713. serge-sans-paille added a comment. Remove `empty()` method and only keep `hasAttributes` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116110/new/ https://reviews.llvm.org/D116110 Files: clang/lib/CodeGen/CodeGenModule.cpp llv

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille updated this revision to Diff 333810. serge-sans-paille added a comment. Adress final(?) comments from @rsmith and @aaron.ballman : Don't warn on argument (or template argument) of top-level decl. Extra test cases Return an enumeration as part of the test CHANGES SINCE LAST ACT

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille marked 10 inline comments as done. serge-sans-paille added inline comments. Comment at: clang/include/clang/AST/Decl.h:82 +enum ReservedIdentifierReason { + StartsWithUnderscoreAtGlobalScope, + StartsWithDoubleUnderscore, aaron.ballman wrote:

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille updated this revision to Diff 333812. serge-sans-paille added a comment. Fix typo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93095/new/ https://reviews.llvm.org/D93095 Files: clang/include/clang/AST/Decl.h clang/include/clang/Basic/DiagnosticGroups.td clang/i

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille updated this revision to Diff 334261. serge-sans-paille marked 10 inline comments as done. serge-sans-paille added a comment. Address all the needed changes pointed out by @aaron.ballman *except* the most critical one on the call to warnOnReservedIdentifier being spread at sever

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:13640 + warnOnReservedIdentifier(New); + aaron.ballman wrote: > rsmith wrote: > > serge-sans-paille wrote: > > > serge-sans-paille wrote: > > > > rsmith wrote: > > > > > Is there

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille updated this revision to Diff 334445. serge-sans-paille added a comment. Herald added a subscriber: dexonsmith. Another round of review :-) I addressed both the scattering of `warnOnReservedIdentifier` and the code split between `Decl.cpp` and `IdentifierInfo.cpp`. CHANGES SI

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille added inline comments. Comment at: clang/lib/AST/Decl.cpp:1097 + // ignored values) that we don't warn on it. + if (Name.size() <= 1) +return ReservedIdentifierStatus::NotReserved; rsmith wrote: > Would it make sense to move the rest of th

[PATCH] D99715: [CMake] Respect LLVM_MINIMUM_PYTHON_VERSION in Tooling/CMakeLists.txt

serge-sans-paille added a comment. Did you try just removing that `find_package`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99715/new/ https://reviews.llvm.org/D99715 ___ cfe-commits mailing list cfe

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille updated this revision to Diff 334660. serge-sans-paille added a comment. Do not use lexical parent, as suggested by @rsmith Add test case for extern function worward-declared in function body, as suggested by @rsmith CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93095

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille updated this revision to Diff 334662. serge-sans-paille added a comment. Warn on friend functions. I failed to support friend classes, but they are only declared and not defined, so that should be fine, right? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93095/new/ h

[PATCH] D99715: [CMake] Fix Python 3 lookup when building LLVM with tests

serge-sans-paille accepted this revision. serge-sans-paille 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/D99715/new/ https://reviews.llvm.org/D99715 __

[PATCH] D60380: Also document -arch as -arch is mac specific

serge-sans-paille added a comment. Reading through the source, it's relatively clear that `-arch` is specific to the Mach0 format, but -target seems applicable to OSX too, see https://godbolt.org/z/j8jvjbo84 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60380/ne

[PATCH] D99409: [clang] Speedup line offset mapping computation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6951b72334bb: [clang] Speedup line offset mapping computation (authored by serge-sans-paille). Herald added a project: clang. Herald added a subscrib

[PATCH] D99409: [clang] Speedup line offset mapping computation

serge-sans-paille added a comment. Thanks @thakis, inestigating the issue is likely to take some time as it seems to be arch or system dependent Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99409/new/ https://reviews.llvm.org/D99409

[PATCH] D99409: [clang] Speedup line offset mapping computation

serge-sans-paille added inline comments. Comment at: clang/lib/Basic/SourceManager.cpp:1262 + unsigned char n) { + return ((x - ~0UL / 255 * (n + 1)) & ~x & + (x & ~0UL / 255 * 127) + ~0UL / 255 * (127 - (m - 1))) &

[PATCH] D99299: Normalize interaction with boolean attributes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. serge-sans-paille marked an inline comment as done. Closed by commit rGd6de1e1a7140: Normalize interaction with boolean attributes (authored by serge-sans-paille). Hera

[PATCH] D99299: Normalize interaction with boolean attributes

serge-sans-paille added inline comments. Comment at: llvm/lib/IR/Attributes.cpp:660-663 +bool AttributeImpl::getValueAsBool() const { + assert(getValueAsString().empty() || getValueAsString() == "false" || getValueAsString() == "true"); + return getValueAsString() == "true"; +

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille updated this revision to Diff 314329. serge-sans-paille added a comment. Rebased on main. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93095/new/ https://reviews.llvm.org/D93095 Files: clang/include/clang/AST/Decl.h clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille updated this revision to Diff 315313. serge-sans-paille added a comment. Address some of the review CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93095/new/ https://reviews.llvm.org/D93095 Files: clang/include/clang/AST/Decl.h clang/include/clang/Basic/DiagnosticG

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille marked an inline comment as done. serge-sans-paille added inline comments. Comment at: clang/test/Sema/reserved-identifier.c:9 +int _foo() { return 0; }// expected-warning {{'_foo' is a reserved identifier}} + +// This one is explicitly skipped by -Wres

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille updated this revision to Diff 315373. serge-sans-paille added a comment. Update codebase and testbed to reflect recent discussion. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93095/new/ https://reviews.llvm.org/D93095 Files: clang/include/clang/AST/Decl.h clang/

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille marked 2 inline comments as done. serge-sans-paille added inline comments. Comment at: clang/test/Sema/reserved-identifier.cpp:58 +// we skip this one because it's not at top-level. +int _barbatruc; // no-warning +} aaron.ballman wrote: > This i

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille updated this revision to Diff 315390. serge-sans-paille marked 2 inline comments as done. serge-sans-paille added a comment. Ignore forward declaration of tagdecl CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93095/new/ https://reviews.llvm.org/D93095 Files: clang/i

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille added inline comments. Comment at: clang/lib/AST/Decl.cpp:1099 +const DeclContext *CurrentContext = getDeclContext(); +while (true) { + if (isa(CurrentContext)) aaron.ballman wrote: > Rather than trying to manually decide whether th

[PATCH] D93095: Introduce -Wreserved-identifier

serge-sans-paille updated this revision to Diff 317677. serge-sans-paille added a comment. As suggested by @aaron.ballman base the detection of top-level-ness on `Sema::LookupName` to avoid re-implementing the wheel. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93095/new/ https://revi

[PATCH] D109967: Simplify handling of builtin with inline redefinition

serge-sans-paille added a comment. In D109967#3044464 , @rnk wrote: > Thanks for doing this, this approach looks like it incorporated my feedback > on the previous approach. Yeah, listened to the wise men... and struggled a lot doing so ;-) =

[PATCH] D112059: Fix inline builtin handling in case of redefinition

serge-sans-paille created this revision. serge-sans-paille added reviewers: nickdesaulniers, manojgupta. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Basically, inline builtin definition are shadowed by externally visib

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

serge-sans-paille added a comment. https://reviews.llvm.org/D112059 should address the issue pointed by @manojgupta and @nickdesaulniers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111009/new/ https://reviews.llvm.org/D111009 __

[PATCH] D112059: Fix inline builtin handling in case of redefinition

serge-sans-paille updated this revision to Diff 381011. serge-sans-paille added a comment. Reduce the number of time we would walk redecls. Simplify test case CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112059/new/ https://reviews.llvm.org/D112059 Files: clang/lib/CodeGen/CodeGenFu

[PATCH] D112059: Fix inline builtin handling in case of redefinition

serge-sans-paille updated this revision to Diff 381014. serge-sans-paille added a comment. Avoid walking redecls CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112059/new/ https://reviews.llvm.org/D112059 Files: clang/lib/CodeGen/CodeGenFunction.cpp clang/test/CodeGen/strlen-inline-

[PATCH] D108571: [clang] allow -fstack-clash-protection on FreeBSD

serge-sans-paille accepted this revision. serge-sans-paille added a comment. This revision is now accepted and ready to land. You know your platform better than I do, and you already ran the patch without any issue, so LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108571/new/ http

[PATCH] D109659: Fix scan-build-py executable lookup path

serge-sans-paille created this revision. serge-sans-paille added reviewers: sylvestre.ledru, isthismyaccount, phosek. Herald added a subscriber: whisperity. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Once installed, s

<    3   4   5   6   7   8   9   >