[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -1822,28 +1822,106 @@ void WriteSemanticSpellingSwitch(const std::string , OS << " }\n"; } +enum class LateAttrParseKind { + LateAttrParsingNever = 0, + LateAttrParsingAlways = 1, + LateAttrParsingExperimentalOnly = 2 +}; + +static LateAttrParseKind getLateAttrParseKind(const Record *Attr) { + // This function basically does + // `Attr->getValueAsDef("LateParsed")->getValueAsInt("Mode")` but does + // a bunch of sanity checking to ensure that + // `LateAttrParseMode` in `Attr.td` is in sync with the `LateAttrParseKind` + // enum in this source file. + + const char *LateParsedStr = "LateParsed"; + auto *LAPK = Attr->getValueAsDef(LateParsedStr); + + // Typecheck the `LateParsed` field. + SmallVector SuperClasses; + LAPK->getDirectSuperClasses(SuperClasses); + if (SuperClasses.size() != 1) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have one super class"); + } + const char *LateAttrParseKindStr = "LateAttrParseKind"; delcypher wrote: I'll give this a try. Guess I'll find out if `llvm::Twine` is happy working with `constexpr`. https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
https://github.com/delcypher updated https://github.com/llvm/llvm-project/pull/88596 >From 554287a724361a510389d7f34f9b57cf434b9657 Mon Sep 17 00:00:00 2001 From: Dan Liew Date: Fri, 12 Apr 2024 17:36:19 -0700 Subject: [PATCH 1/2] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag This patch changes the `LateParsed` field of `Attr` in `Attr.td` to be an instantiation of the new `LateAttrParseKind` class. The instation can be one of the following: * `LateAttrParsingNever` - Corresponds with the false value of `LateParsed` prior to this patch (the default for an attribute). * `LateAttrParsingAlways` - Corresponds with the true value of `LateParsed` prior to this patch. * `LateAttrParsingExperimentalOnly` - A new mode described below. `LateAttrParsingExperimentalOnly` means that the attribute will be late parsed if the new the `ExperimentalLateParseAttributes` language option (also introduced in this patch) is enabled and will **not** be late parsed if the language option is disabled. The new `ExperimentalLateParseAttributes` language option is controlled by a new driver and frontend flag (`-fexperimental-late-parse-attributes`). A driver test is included to check that the driver passes the flag to CC1. In this patch the `LateAttrParsingExperimentalOnly` is not adopted by any attribute so `-fexperimental-late-pase-attributes` and the corresponding language option currently have no effect on compilation. This is why this patch does not include any test cases that test `LateAttrParsingExperimentalOnly`'s behavior. The motivation for this patch is to be able to late parse the new `counted_by` attribute but only do so when a feature flag is passed. This was discussed during the [bounds-safety RFC](https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/70854/68). Adoption of `LateAttrParsingExperimentalOnly` for the `counted_by` attributed will be handled separately in another patch (likely https://github.com/llvm/llvm-project/pull/87596). --- clang/include/clang/Basic/Attr.td | 46 +++--- clang/include/clang/Basic/LangOptions.def | 1 + clang/include/clang/Driver/Options.td | 7 + clang/include/clang/Parse/Parser.h| 4 + clang/lib/Driver/ToolChains/Clang.cpp | 3 + clang/lib/Parse/ParseDecl.cpp | 19 ++- .../experimental-late-parse-attributes.c | 12 ++ clang/utils/TableGen/ClangAttrEmitter.cpp | 137 -- 8 files changed, 192 insertions(+), 37 deletions(-) create mode 100644 clang/test/Driver/experimental-late-parse-attributes.c diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index dc87a8c6f022dc..da76f807cd676c 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -592,6 +592,16 @@ class AttrSubjectMatcherAggregateRule { def SubjectMatcherForNamed : AttrSubjectMatcherAggregateRule; +// Late Attribute parsing mode enum +class LateAttrParseKind { + int Kind = val; +} +def LateAttrParsingNever : LateAttrParseKind<0>; // Never late parsed +def LateAttrParsingAlways: LateAttrParseKind<1>; // Always late parsed +// Late parsed if `-fexperimental-late-parse-attributes` is on and parsed +// normally if off. +def LateAttrParsingExperimentalOnly : LateAttrParseKind<2>; + class Attr { // The various ways in which an attribute can be spelled in source list Spellings; @@ -603,8 +613,8 @@ class Attr { list Accessors = []; // Specify targets for spellings. list TargetSpecificSpellings = []; - // Set to true for attributes with arguments which require delayed parsing. - bit LateParsed = 0; + // Specifies the late parsing kind. + LateAttrParseKind LateParsed = LateAttrParsingNever; // Set to false to prevent an attribute from being propagated from a template // to the instantiation. bit Clone = 1; @@ -3173,7 +3183,7 @@ def DiagnoseIf : InheritableAttr { BoolArgument<"ArgDependent", 0, /*fake*/ 1>, DeclArgument]; let InheritEvenIfAlreadyPresent = 1; - let LateParsed = 1; + let LateParsed = LateAttrParsingAlways; let AdditionalMembers = [{ bool isError() const { return diagnosticType == DT_Error; } bool isWarning() const { return diagnosticType == DT_Warning; } @@ -3472,7 +3482,7 @@ def AssertCapability : InheritableAttr { let Spellings = [Clang<"assert_capability", 0>, Clang<"assert_shared_capability", 0>]; let Subjects = SubjectList<[Function]>; - let LateParsed = 1; + let LateParsed = LateAttrParsingAlways; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3488,7 +3498,7 @@ def AcquireCapability : InheritableAttr { GNU<"exclusive_lock_function">, GNU<"shared_lock_function">]; let Subjects = SubjectList<[Function]>; - let LateParsed = 1;
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
delcypher wrote: @Sirraide Thanks for the feedback. > This should also add some tests that actually use experimentally late-parsed > attributes w/ the flag explicilty enabled/disabled. The intention is for the functionality being added to be used in a subsequent PR. This is currently sketched in this work-in-progress PR (#87596). This patch is separate from #87596 because it makes reviewing easier. I do not know how I would write tests for declaring an attribute as experimentally late parsed without that. AFAIK I'd have to use `LateAttrParsingExperimentalOnly` in `Attr.td` but * It seems undesirable to change any of the existing attributes just for the purposes of testing. * Adding a fake attribute for testing would likely impact Clang itself (i.e. the fake attribute would show up as something clang would parse) which doesn't seem desirable at all. Given that a subsequent patch will add the necessary test coverage and the above problems I think it is reasonable for explicit tests for `LateAttrParsingExperimentalOnly` to be omitted . https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix name conflict with `sys/mac.h` on AIX (PR #88644)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Vlad Serebrennikov (Endilll) Changes Fixes clang-ppc64-aix bot failure after #88559 (0a6f6df5b0c3d0f2a42f013bf5cafb9b5020dcac) https://lab.llvm.org/buildbot/#/builders/214/builds/11887 --- Full diff: https://github.com/llvm/llvm-project/pull/88644.diff 1 Files Affected: - (modified) clang/include/clang/Basic/Cuda.h (+5) ``diff diff --git a/clang/include/clang/Basic/Cuda.h b/clang/include/clang/Basic/Cuda.h index acc6bb6581d857..3908e10bc61061 100644 --- a/clang/include/clang/Basic/Cuda.h +++ b/clang/include/clang/Basic/Cuda.h @@ -50,6 +50,11 @@ const char *CudaVersionToString(CudaVersion V); // Input is "Major.Minor" CudaVersion CudaStringToVersion(const llvm::Twine ); +// We have a name conflict with sys/mac.h on AIX +#ifdef _AIX +#undef SM_32 +#endif + enum class CudaArch { UNUSED, UNKNOWN, `` https://github.com/llvm/llvm-project/pull/88644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix name conflict with `sys/mac.h` on AIX (PR #88644)
https://github.com/Endilll created https://github.com/llvm/llvm-project/pull/88644 Fixes clang-ppc64-aix bot failure after #88559 (0a6f6df5b0c3d0f2a42f013bf5cafb9b5020dcac) https://lab.llvm.org/buildbot/#/builders/214/builds/11887 >From 9d46b1ed31d2acbb772f9bb4b139fa1ec36a65ab Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov Date: Sun, 14 Apr 2024 08:46:27 +0300 Subject: [PATCH] [clang] Fix name conflict with `sys/mac.h` on AIX Fixes clang-ppc64-aix bot failure after #88559 (0a6f6df5b0c3d0f2a42f013bf5cafb9b5020dcac) https://lab.llvm.org/buildbot/#/builders/214/builds/11887 --- clang/include/clang/Basic/Cuda.h | 5 + 1 file changed, 5 insertions(+) diff --git a/clang/include/clang/Basic/Cuda.h b/clang/include/clang/Basic/Cuda.h index acc6bb6581d857..3908e10bc61061 100644 --- a/clang/include/clang/Basic/Cuda.h +++ b/clang/include/clang/Basic/Cuda.h @@ -50,6 +50,11 @@ const char *CudaVersionToString(CudaVersion V); // Input is "Major.Minor" CudaVersion CudaStringToVersion(const llvm::Twine ); +// We have a name conflict with sys/mac.h on AIX +#ifdef _AIX +#undef SM_32 +#endif + enum class CudaArch { UNUSED, UNKNOWN, ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -2101,9 +2179,20 @@ bool PragmaClangAttributeSupport::isAttributedSupported( return SpecifiedResult; // Opt-out rules: - // An attribute requires delayed parsing (LateParsed is on) - if (Attribute.getValueAsBit("LateParsed")) + + // An attribute requires delayed parsing (LateParsed is on). + switch (getLateAttrParseKind()) { + case LateAttrParseKind::LateAttrParsingNever: +break; + case LateAttrParseKind::LateAttrParsingAlways: +return false; + case LateAttrParseKind::LateAttrParsingExperimentalOnly: +// FIXME: This is only late parsed when +// `-fexperimental-late-parse-attributes` is on. If that option is off +// then we shouldn't return here. return false; Sirraide wrote: I’d leave a comment here explaining the situation; I’m just saying that I’m not convinced there really is any ‘fixing’ this here w/o making the code around `IsSupportedByPragmaAttribute` aware of this, which seems like a strange thing to do. https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -1822,28 +1822,106 @@ void WriteSemanticSpellingSwitch(const std::string , OS << " }\n"; } +enum class LateAttrParseKind { + LateAttrParsingNever = 0, + LateAttrParsingAlways = 1, + LateAttrParsingExperimentalOnly = 2 +}; + +static LateAttrParseKind getLateAttrParseKind(const Record *Attr) { + // This function basically does + // `Attr->getValueAsDef("LateParsed")->getValueAsInt("Mode")` but does + // a bunch of sanity checking to ensure that + // `LateAttrParseMode` in `Attr.td` is in sync with the `LateAttrParseKind` + // enum in this source file. + + const char *LateParsedStr = "LateParsed"; + auto *LAPK = Attr->getValueAsDef(LateParsedStr); + + // Typecheck the `LateParsed` field. + SmallVector SuperClasses; + LAPK->getDirectSuperClasses(SuperClasses); + if (SuperClasses.size() != 1) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have one super class"); + } + const char *LateAttrParseKindStr = "LateAttrParseKind"; + if (SuperClasses[0]->getName().compare(LateAttrParseKindStr) != 0) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have type `" + + llvm::Twine(LateAttrParseKindStr) + + "` but found type `" + + SuperClasses[0]->getName() + "`"); + } + + // Get Kind and verify the enum name matches the name in `Attr.td`. + const char *KindFieldStr = "Kind"; + unsigned Kind = LAPK->getValueAsInt(KindFieldStr); + switch (LateAttrParseKind(Kind)) { +#define CASE(X) \ + case LateAttrParseKind::X: \ +if (LAPK->getName().compare(#X) != 0) { \ + PrintFatalError(Attr, \ + "Field `" + llvm::Twine(LateParsedStr) + "` set to `" + \ + LAPK->getName() + \ + "` but this converts to `LateAttrParseKind::" + \ + llvm::Twine(#X) + "`"); \ +} \ +return LateAttrParseKind::X; + +CASE(LateAttrParsingNever) +CASE(LateAttrParsingAlways) +CASE(LateAttrParsingExperimentalOnly) +#undef CASE + } + + // The Kind value is completely invalid + auto KindValueStr = llvm::utostr(Kind); + PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + "` set to `" + +LAPK->getName() + "` has unexpected `" + +llvm::Twine(KindFieldStr) + "` value of " + +KindValueStr); +} + // Emits the LateParsed property for attributes. -static void emitClangAttrLateParsedList(RecordKeeper , raw_ostream ) { - OS << "#if defined(CLANG_ATTR_LATE_PARSED_LIST)\n"; - std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); +static void emitClangAttrLateParsedListImpl(RecordKeeper , +raw_ostream , +LateAttrParseKind LateParseMode) { + std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); for (const auto *Attr : Attrs) { -bool LateParsed = Attr->getValueAsBit("LateParsed"); +auto LateParsed = getLateAttrParseKind(Attr); -if (LateParsed) { - std::vector Spellings = GetFlattenedSpellings(*Attr); +if (LateParsed != LateParseMode) + continue; - // FIXME: Handle non-GNU attributes - for (const auto : Spellings) { -if (I.variety() != "GNU") - continue; -OS << ".Case(\"" << I.name() << "\", " << LateParsed << ")\n"; - } +std::vector Spellings = GetFlattenedSpellings(*Attr); + +// FIXME: Handle non-GNU attributes +for (const auto : Spellings) { + if (I.variety() != "GNU") +continue; + OS << ".Case(\"" << I.name() << "\", 1)\n"; } } +} + +static void emitClangAttrLateParsedList(RecordKeeper , +raw_ostream ) { + OS << "#if defined(CLANG_ATTR_LATE_PARSED_LIST)\n"; + emitClangAttrLateParsedListImpl(Records, OS, + LateAttrParseKind::LateAttrParsingAlways); OS << "#endif // CLANG_ATTR_LATE_PARSED_LIST\n\n"; } Sirraide wrote: Oh, my bad, I somehow thought that there were two different ‘Impl’ functions. It makes sense if it’s the same function being called twice. https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -2101,9 +2179,20 @@ bool PragmaClangAttributeSupport::isAttributedSupported( return SpecifiedResult; // Opt-out rules: - // An attribute requires delayed parsing (LateParsed is on) - if (Attribute.getValueAsBit("LateParsed")) + + // An attribute requires delayed parsing (LateParsed is on). + switch (getLateAttrParseKind()) { + case LateAttrParseKind::LateAttrParsingNever: +break; + case LateAttrParseKind::LateAttrParsingAlways: +return false; + case LateAttrParseKind::LateAttrParsingExperimentalOnly: +// FIXME: This is only late parsed when +// `-fexperimental-late-parse-attributes` is on. If that option is off +// then we shouldn't return here. return false; delcypher wrote: So you're saying I should drop the `FIXME`? https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -1822,28 +1822,106 @@ void WriteSemanticSpellingSwitch(const std::string , OS << " }\n"; } +enum class LateAttrParseKind { + LateAttrParsingNever = 0, + LateAttrParsingAlways = 1, + LateAttrParsingExperimentalOnly = 2 +}; + +static LateAttrParseKind getLateAttrParseKind(const Record *Attr) { + // This function basically does + // `Attr->getValueAsDef("LateParsed")->getValueAsInt("Mode")` but does + // a bunch of sanity checking to ensure that + // `LateAttrParseMode` in `Attr.td` is in sync with the `LateAttrParseKind` + // enum in this source file. + + const char *LateParsedStr = "LateParsed"; + auto *LAPK = Attr->getValueAsDef(LateParsedStr); + + // Typecheck the `LateParsed` field. + SmallVector SuperClasses; + LAPK->getDirectSuperClasses(SuperClasses); + if (SuperClasses.size() != 1) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have one super class"); + } + const char *LateAttrParseKindStr = "LateAttrParseKind"; + if (SuperClasses[0]->getName().compare(LateAttrParseKindStr) != 0) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have type `" + + llvm::Twine(LateAttrParseKindStr) + + "` but found type `" + + SuperClasses[0]->getName() + "`"); + } + + // Get Kind and verify the enum name matches the name in `Attr.td`. + const char *KindFieldStr = "Kind"; + unsigned Kind = LAPK->getValueAsInt(KindFieldStr); + switch (LateAttrParseKind(Kind)) { +#define CASE(X) \ + case LateAttrParseKind::X: \ +if (LAPK->getName().compare(#X) != 0) { \ + PrintFatalError(Attr, \ + "Field `" + llvm::Twine(LateParsedStr) + "` set to `" + \ + LAPK->getName() + \ + "` but this converts to `LateAttrParseKind::" + \ + llvm::Twine(#X) + "`"); \ +} \ +return LateAttrParseKind::X; + +CASE(LateAttrParsingNever) +CASE(LateAttrParsingAlways) +CASE(LateAttrParsingExperimentalOnly) +#undef CASE + } + + // The Kind value is completely invalid + auto KindValueStr = llvm::utostr(Kind); + PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + "` set to `" + +LAPK->getName() + "` has unexpected `" + +llvm::Twine(KindFieldStr) + "` value of " + +KindValueStr); +} + // Emits the LateParsed property for attributes. -static void emitClangAttrLateParsedList(RecordKeeper , raw_ostream ) { - OS << "#if defined(CLANG_ATTR_LATE_PARSED_LIST)\n"; - std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); +static void emitClangAttrLateParsedListImpl(RecordKeeper , +raw_ostream , +LateAttrParseKind LateParseMode) { + std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); for (const auto *Attr : Attrs) { -bool LateParsed = Attr->getValueAsBit("LateParsed"); +auto LateParsed = getLateAttrParseKind(Attr); -if (LateParsed) { - std::vector Spellings = GetFlattenedSpellings(*Attr); +if (LateParsed != LateParseMode) + continue; - // FIXME: Handle non-GNU attributes - for (const auto : Spellings) { -if (I.variety() != "GNU") - continue; -OS << ".Case(\"" << I.name() << "\", " << LateParsed << ")\n"; - } +std::vector Spellings = GetFlattenedSpellings(*Attr); + +// FIXME: Handle non-GNU attributes +for (const auto : Spellings) { + if (I.variety() != "GNU") +continue; + OS << ".Case(\"" << I.name() << "\", 1)\n"; } } +} + +static void emitClangAttrLateParsedList(RecordKeeper , +raw_ostream ) { + OS << "#if defined(CLANG_ATTR_LATE_PARSED_LIST)\n"; + emitClangAttrLateParsedListImpl(Records, OS, + LateAttrParseKind::LateAttrParsingAlways); OS << "#endif // CLANG_ATTR_LATE_PARSED_LIST\n\n"; } delcypher wrote: Or did you mean something else? https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -1822,28 +1822,106 @@ void WriteSemanticSpellingSwitch(const std::string , OS << " }\n"; } +enum class LateAttrParseKind { + LateAttrParsingNever = 0, + LateAttrParsingAlways = 1, + LateAttrParsingExperimentalOnly = 2 +}; + +static LateAttrParseKind getLateAttrParseKind(const Record *Attr) { + // This function basically does + // `Attr->getValueAsDef("LateParsed")->getValueAsInt("Mode")` but does + // a bunch of sanity checking to ensure that + // `LateAttrParseMode` in `Attr.td` is in sync with the `LateAttrParseKind` + // enum in this source file. + + const char *LateParsedStr = "LateParsed"; + auto *LAPK = Attr->getValueAsDef(LateParsedStr); + + // Typecheck the `LateParsed` field. + SmallVector SuperClasses; + LAPK->getDirectSuperClasses(SuperClasses); + if (SuperClasses.size() != 1) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have one super class"); + } + const char *LateAttrParseKindStr = "LateAttrParseKind"; + if (SuperClasses[0]->getName().compare(LateAttrParseKindStr) != 0) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have type `" + + llvm::Twine(LateAttrParseKindStr) + + "` but found type `" + + SuperClasses[0]->getName() + "`"); + } + + // Get Kind and verify the enum name matches the name in `Attr.td`. + const char *KindFieldStr = "Kind"; + unsigned Kind = LAPK->getValueAsInt(KindFieldStr); + switch (LateAttrParseKind(Kind)) { +#define CASE(X) \ + case LateAttrParseKind::X: \ +if (LAPK->getName().compare(#X) != 0) { \ + PrintFatalError(Attr, \ + "Field `" + llvm::Twine(LateParsedStr) + "` set to `" + \ + LAPK->getName() + \ + "` but this converts to `LateAttrParseKind::" + \ + llvm::Twine(#X) + "`"); \ +} \ +return LateAttrParseKind::X; + +CASE(LateAttrParsingNever) +CASE(LateAttrParsingAlways) +CASE(LateAttrParsingExperimentalOnly) +#undef CASE + } + + // The Kind value is completely invalid + auto KindValueStr = llvm::utostr(Kind); + PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + "` set to `" + +LAPK->getName() + "` has unexpected `" + +llvm::Twine(KindFieldStr) + "` value of " + +KindValueStr); +} + // Emits the LateParsed property for attributes. -static void emitClangAttrLateParsedList(RecordKeeper , raw_ostream ) { - OS << "#if defined(CLANG_ATTR_LATE_PARSED_LIST)\n"; - std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); +static void emitClangAttrLateParsedListImpl(RecordKeeper , +raw_ostream , +LateAttrParseKind LateParseMode) { + std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); for (const auto *Attr : Attrs) { -bool LateParsed = Attr->getValueAsBit("LateParsed"); +auto LateParsed = getLateAttrParseKind(Attr); -if (LateParsed) { - std::vector Spellings = GetFlattenedSpellings(*Attr); +if (LateParsed != LateParseMode) + continue; - // FIXME: Handle non-GNU attributes - for (const auto : Spellings) { -if (I.variety() != "GNU") - continue; -OS << ".Case(\"" << I.name() << "\", " << LateParsed << ")\n"; - } +std::vector Spellings = GetFlattenedSpellings(*Attr); + +// FIXME: Handle non-GNU attributes +for (const auto : Spellings) { + if (I.variety() != "GNU") +continue; + OS << ".Case(\"" << I.name() << "\", 1)\n"; } } +} + +static void emitClangAttrLateParsedList(RecordKeeper , +raw_ostream ) { + OS << "#if defined(CLANG_ATTR_LATE_PARSED_LIST)\n"; + emitClangAttrLateParsedListImpl(Records, OS, + LateAttrParseKind::LateAttrParsingAlways); OS << "#endif // CLANG_ATTR_LATE_PARSED_LIST\n\n"; } delcypher wrote: By "inling" I assume you mean duplicating `emitClangAttrLateParsedListImpl` into `emitClangAttrLateParsedList` and `emitClangAttrLateParsedExperimentalList`. I've deliberately avoided doing that to avoid duplicating code. Another reason for avoiding the duplication is that the code has a `FIXME` and its probably best to avoid duplicating that if it's not necessary. https://github.com/llvm/llvm-project/pull/88596
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -1822,28 +1822,106 @@ void WriteSemanticSpellingSwitch(const std::string , OS << " }\n"; } +enum class LateAttrParseKind { + LateAttrParsingNever = 0, + LateAttrParsingAlways = 1, + LateAttrParsingExperimentalOnly = 2 +}; + Sirraide wrote: If it’s just for the macro here, then one option would also be to use token pasting to concatenate `LateAttrParsing` and the name of the enumerator. https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -1822,28 +1822,106 @@ void WriteSemanticSpellingSwitch(const std::string , OS << " }\n"; } +enum class LateAttrParseKind { + LateAttrParsingNever = 0, + LateAttrParsingAlways = 1, + LateAttrParsingExperimentalOnly = 2 +}; + delcypher wrote: Yes. The current implementation relies on the name of the enum value and the name in `Attr.td` file being the same for the purposes of verifying the enum is in sync with the corresponding definition in the `Attr.td` file. You can see this here: ```c++ #define CASE(X)\ case LateAttrParseKind::X: \ if (LAPK->getName().compare(#X) != 0) { ``` `LAPK->getName()` is the name of the definition in the `Attr.td` file and `X` in the macro if the enum name in the `LateAttrParseKind`. https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Introduce `SemaCUDA` (PR #88559)
Endilll wrote: I think I exposed a name conflict with a system header on AIX by including `Cuda.h` in `Sema.h`. Given that this name conflict potentially affects both enum definition and its usage, how should we resolve this? https://github.com/llvm/llvm-project/pull/88559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -1822,28 +1822,106 @@ void WriteSemanticSpellingSwitch(const std::string , OS << " }\n"; } +enum class LateAttrParseKind { + LateAttrParsingNever = 0, + LateAttrParsingAlways = 1, + LateAttrParsingExperimentalOnly = 2 +}; + +static LateAttrParseKind getLateAttrParseKind(const Record *Attr) { + // This function basically does + // `Attr->getValueAsDef("LateParsed")->getValueAsInt("Mode")` but does + // a bunch of sanity checking to ensure that + // `LateAttrParseMode` in `Attr.td` is in sync with the `LateAttrParseKind` + // enum in this source file. + + const char *LateParsedStr = "LateParsed"; + auto *LAPK = Attr->getValueAsDef(LateParsedStr); + + // Typecheck the `LateParsed` field. + SmallVector SuperClasses; + LAPK->getDirectSuperClasses(SuperClasses); + if (SuperClasses.size() != 1) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have one super class"); + } + const char *LateAttrParseKindStr = "LateAttrParseKind"; + if (SuperClasses[0]->getName().compare(LateAttrParseKindStr) != 0) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have type `" + + llvm::Twine(LateAttrParseKindStr) + + "` but found type `" + + SuperClasses[0]->getName() + "`"); + } + + // Get Kind and verify the enum name matches the name in `Attr.td`. + const char *KindFieldStr = "Kind"; Sirraide wrote: ```suggestion static constexpr StringRef KindFieldStr = "Kind"; ``` https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -2885,8 +2974,23 @@ static void emitAttributes(RecordKeeper , raw_ostream , return; } OS << "\n : " << SuperName << "(Ctx, CommonInfo, "; - OS << "attr::" << R.getName() << ", " - << (R.getValueAsBit("LateParsed") ? "true" : "false"); + OS << "attr::" << R.getName() << ", "; + + // Handle different late parsing modes. + switch (getLateAttrParseKind()) { + case LateAttrParseKind::LateAttrParsingNever: +OS << '0'; +break; + case LateAttrParseKind::LateAttrParsingAlways: +OS << '1'; Sirraide wrote: ```suggestion OS << "true"; ``` https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -1822,28 +1822,106 @@ void WriteSemanticSpellingSwitch(const std::string , OS << " }\n"; } +enum class LateAttrParseKind { + LateAttrParsingNever = 0, + LateAttrParsingAlways = 1, + LateAttrParsingExperimentalOnly = 2 +}; + +static LateAttrParseKind getLateAttrParseKind(const Record *Attr) { + // This function basically does + // `Attr->getValueAsDef("LateParsed")->getValueAsInt("Mode")` but does + // a bunch of sanity checking to ensure that + // `LateAttrParseMode` in `Attr.td` is in sync with the `LateAttrParseKind` + // enum in this source file. + + const char *LateParsedStr = "LateParsed"; + auto *LAPK = Attr->getValueAsDef(LateParsedStr); + + // Typecheck the `LateParsed` field. + SmallVector SuperClasses; + LAPK->getDirectSuperClasses(SuperClasses); + if (SuperClasses.size() != 1) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have one super class"); + } + const char *LateAttrParseKindStr = "LateAttrParseKind"; + if (SuperClasses[0]->getName().compare(LateAttrParseKindStr) != 0) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have type `" + + llvm::Twine(LateAttrParseKindStr) + + "` but found type `" + + SuperClasses[0]->getName() + "`"); + } + + // Get Kind and verify the enum name matches the name in `Attr.td`. + const char *KindFieldStr = "Kind"; + unsigned Kind = LAPK->getValueAsInt(KindFieldStr); + switch (LateAttrParseKind(Kind)) { +#define CASE(X) \ + case LateAttrParseKind::X: \ +if (LAPK->getName().compare(#X) != 0) { \ + PrintFatalError(Attr, \ + "Field `" + llvm::Twine(LateParsedStr) + "` set to `" + \ + LAPK->getName() + \ + "` but this converts to `LateAttrParseKind::" + \ + llvm::Twine(#X) + "`"); \ +} \ +return LateAttrParseKind::X; + +CASE(LateAttrParsingNever) +CASE(LateAttrParsingAlways) +CASE(LateAttrParsingExperimentalOnly) +#undef CASE + } + + // The Kind value is completely invalid + auto KindValueStr = llvm::utostr(Kind); + PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + "` set to `" + +LAPK->getName() + "` has unexpected `" + +llvm::Twine(KindFieldStr) + "` value of " + +KindValueStr); +} + // Emits the LateParsed property for attributes. -static void emitClangAttrLateParsedList(RecordKeeper , raw_ostream ) { - OS << "#if defined(CLANG_ATTR_LATE_PARSED_LIST)\n"; - std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); +static void emitClangAttrLateParsedListImpl(RecordKeeper , +raw_ostream , +LateAttrParseKind LateParseMode) { + std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); for (const auto *Attr : Attrs) { -bool LateParsed = Attr->getValueAsBit("LateParsed"); +auto LateParsed = getLateAttrParseKind(Attr); -if (LateParsed) { - std::vector Spellings = GetFlattenedSpellings(*Attr); +if (LateParsed != LateParseMode) + continue; - // FIXME: Handle non-GNU attributes - for (const auto : Spellings) { -if (I.variety() != "GNU") - continue; -OS << ".Case(\"" << I.name() << "\", " << LateParsed << ")\n"; - } +std::vector Spellings = GetFlattenedSpellings(*Attr); + +// FIXME: Handle non-GNU attributes +for (const auto : Spellings) { + if (I.variety() != "GNU") +continue; + OS << ".Case(\"" << I.name() << "\", 1)\n"; } } +} + +static void emitClangAttrLateParsedList(RecordKeeper , +raw_ostream ) { + OS << "#if defined(CLANG_ATTR_LATE_PARSED_LIST)\n"; + emitClangAttrLateParsedListImpl(Records, OS, + LateAttrParseKind::LateAttrParsingAlways); OS << "#endif // CLANG_ATTR_LATE_PARSED_LIST\n\n"; } +static void emitClangAttrLateParsedExperimentalList(RecordKeeper , Sirraide wrote: Same here https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -2885,8 +2974,23 @@ static void emitAttributes(RecordKeeper , raw_ostream , return; } OS << "\n : " << SuperName << "(Ctx, CommonInfo, "; - OS << "attr::" << R.getName() << ", " - << (R.getValueAsBit("LateParsed") ? "true" : "false"); + OS << "attr::" << R.getName() << ", "; + + // Handle different late parsing modes. + switch (getLateAttrParseKind()) { + case LateAttrParseKind::LateAttrParsingNever: +OS << '0'; Sirraide wrote: ```suggestion OS << "false"; ``` https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -1822,28 +1822,106 @@ void WriteSemanticSpellingSwitch(const std::string , OS << " }\n"; } +enum class LateAttrParseKind { + LateAttrParsingNever = 0, + LateAttrParsingAlways = 1, + LateAttrParsingExperimentalOnly = 2 +}; + +static LateAttrParseKind getLateAttrParseKind(const Record *Attr) { + // This function basically does + // `Attr->getValueAsDef("LateParsed")->getValueAsInt("Mode")` but does + // a bunch of sanity checking to ensure that + // `LateAttrParseMode` in `Attr.td` is in sync with the `LateAttrParseKind` + // enum in this source file. + + const char *LateParsedStr = "LateParsed"; + auto *LAPK = Attr->getValueAsDef(LateParsedStr); + + // Typecheck the `LateParsed` field. + SmallVector SuperClasses; + LAPK->getDirectSuperClasses(SuperClasses); + if (SuperClasses.size() != 1) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have one super class"); + } Sirraide wrote: ```suggestion if (SuperClasses.size() != 1) PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + "`should only have one super class"); ``` The body of the `if` is small enough that we don’t need the `{}`. https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -1822,28 +1822,106 @@ void WriteSemanticSpellingSwitch(const std::string , OS << " }\n"; } +enum class LateAttrParseKind { + LateAttrParsingNever = 0, + LateAttrParsingAlways = 1, + LateAttrParsingExperimentalOnly = 2 +}; + +static LateAttrParseKind getLateAttrParseKind(const Record *Attr) { + // This function basically does + // `Attr->getValueAsDef("LateParsed")->getValueAsInt("Mode")` but does + // a bunch of sanity checking to ensure that + // `LateAttrParseMode` in `Attr.td` is in sync with the `LateAttrParseKind` + // enum in this source file. + + const char *LateParsedStr = "LateParsed"; + auto *LAPK = Attr->getValueAsDef(LateParsedStr); + + // Typecheck the `LateParsed` field. + SmallVector SuperClasses; + LAPK->getDirectSuperClasses(SuperClasses); + if (SuperClasses.size() != 1) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have one super class"); + } + const char *LateAttrParseKindStr = "LateAttrParseKind"; + if (SuperClasses[0]->getName().compare(LateAttrParseKindStr) != 0) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have type `" + + llvm::Twine(LateAttrParseKindStr) + + "` but found type `" + + SuperClasses[0]->getName() + "`"); + } + + // Get Kind and verify the enum name matches the name in `Attr.td`. + const char *KindFieldStr = "Kind"; + unsigned Kind = LAPK->getValueAsInt(KindFieldStr); + switch (LateAttrParseKind(Kind)) { +#define CASE(X) \ + case LateAttrParseKind::X: \ +if (LAPK->getName().compare(#X) != 0) { \ + PrintFatalError(Attr, \ + "Field `" + llvm::Twine(LateParsedStr) + "` set to `" + \ + LAPK->getName() + \ + "` but this converts to `LateAttrParseKind::" + \ + llvm::Twine(#X) + "`"); \ +} \ +return LateAttrParseKind::X; + +CASE(LateAttrParsingNever) +CASE(LateAttrParsingAlways) +CASE(LateAttrParsingExperimentalOnly) +#undef CASE + } + + // The Kind value is completely invalid + auto KindValueStr = llvm::utostr(Kind); + PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + "` set to `" + +LAPK->getName() + "` has unexpected `" + +llvm::Twine(KindFieldStr) + "` value of " + +KindValueStr); +} + // Emits the LateParsed property for attributes. -static void emitClangAttrLateParsedList(RecordKeeper , raw_ostream ) { - OS << "#if defined(CLANG_ATTR_LATE_PARSED_LIST)\n"; - std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); +static void emitClangAttrLateParsedListImpl(RecordKeeper , +raw_ostream , +LateAttrParseKind LateParseMode) { + std::vector Attrs = Records.getAllDerivedDefinitions("Attr"); for (const auto *Attr : Attrs) { -bool LateParsed = Attr->getValueAsBit("LateParsed"); +auto LateParsed = getLateAttrParseKind(Attr); -if (LateParsed) { - std::vector Spellings = GetFlattenedSpellings(*Attr); +if (LateParsed != LateParseMode) + continue; - // FIXME: Handle non-GNU attributes - for (const auto : Spellings) { -if (I.variety() != "GNU") - continue; -OS << ".Case(\"" << I.name() << "\", " << LateParsed << ")\n"; - } +std::vector Spellings = GetFlattenedSpellings(*Attr); + +// FIXME: Handle non-GNU attributes +for (const auto : Spellings) { + if (I.variety() != "GNU") +continue; + OS << ".Case(\"" << I.name() << "\", 1)\n"; } } +} + +static void emitClangAttrLateParsedList(RecordKeeper , +raw_ostream ) { + OS << "#if defined(CLANG_ATTR_LATE_PARSED_LIST)\n"; + emitClangAttrLateParsedListImpl(Records, OS, + LateAttrParseKind::LateAttrParsingAlways); OS << "#endif // CLANG_ATTR_LATE_PARSED_LIST\n\n"; } Sirraide wrote: Imo the code in `emit...Impl()` can just be inlined into this since it doesn’t have any early `return`s or anything like that anyway. https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -2101,9 +2179,20 @@ bool PragmaClangAttributeSupport::isAttributedSupported( return SpecifiedResult; // Opt-out rules: - // An attribute requires delayed parsing (LateParsed is on) - if (Attribute.getValueAsBit("LateParsed")) + + // An attribute requires delayed parsing (LateParsed is on). + switch (getLateAttrParseKind()) { + case LateAttrParseKind::LateAttrParsingNever: +break; + case LateAttrParseKind::LateAttrParsingAlways: +return false; + case LateAttrParseKind::LateAttrParsingExperimentalOnly: +// FIXME: This is only late parsed when +// `-fexperimental-late-parse-attributes` is on. If that option is off +// then we shouldn't return here. return false; Sirraide wrote: Hmm, I’d probably just keep returning `false` here as there is no good way to handle this here—at least not that I can think of. Not supporting it here if it may sometimes be late parsed is probably fine. https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -1822,28 +1822,106 @@ void WriteSemanticSpellingSwitch(const std::string , OS << " }\n"; } +enum class LateAttrParseKind { + LateAttrParsingNever = 0, + LateAttrParsingAlways = 1, + LateAttrParsingExperimentalOnly = 2 +}; + +static LateAttrParseKind getLateAttrParseKind(const Record *Attr) { + // This function basically does + // `Attr->getValueAsDef("LateParsed")->getValueAsInt("Mode")` but does + // a bunch of sanity checking to ensure that + // `LateAttrParseMode` in `Attr.td` is in sync with the `LateAttrParseKind` + // enum in this source file. + + const char *LateParsedStr = "LateParsed"; Sirraide wrote: ```suggestion static constexpr StringRef LateParsedStr = "LateParsed"; ``` https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -1822,28 +1822,106 @@ void WriteSemanticSpellingSwitch(const std::string , OS << " }\n"; } +enum class LateAttrParseKind { + LateAttrParsingNever = 0, + LateAttrParsingAlways = 1, + LateAttrParsingExperimentalOnly = 2 +}; + Sirraide wrote: This seems a bit verbose seeing as it’s a scoped enum anyway. Is there a reason why these aren’t just called `Never` etc. https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -1822,28 +1822,106 @@ void WriteSemanticSpellingSwitch(const std::string , OS << " }\n"; } +enum class LateAttrParseKind { + LateAttrParsingNever = 0, + LateAttrParsingAlways = 1, + LateAttrParsingExperimentalOnly = 2 +}; + +static LateAttrParseKind getLateAttrParseKind(const Record *Attr) { + // This function basically does + // `Attr->getValueAsDef("LateParsed")->getValueAsInt("Mode")` but does + // a bunch of sanity checking to ensure that + // `LateAttrParseMode` in `Attr.td` is in sync with the `LateAttrParseKind` + // enum in this source file. + + const char *LateParsedStr = "LateParsed"; + auto *LAPK = Attr->getValueAsDef(LateParsedStr); + + // Typecheck the `LateParsed` field. + SmallVector SuperClasses; + LAPK->getDirectSuperClasses(SuperClasses); + if (SuperClasses.size() != 1) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have one super class"); + } + const char *LateAttrParseKindStr = "LateAttrParseKind"; Sirraide wrote: ```suggestion static constexpr StringRef LateAttrParseKindStr = "LateAttrParseKind"; ``` https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
https://github.com/Sirraide requested changes to this pull request. Not too familiar w/ the overall context of this, so I can only comment on the implementation. This should also add some tests that actually use experimentally late-parsed attributes w/ the flag explicilty enabled/disabled. https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
@@ -1822,28 +1822,106 @@ void WriteSemanticSpellingSwitch(const std::string , OS << " }\n"; } +enum class LateAttrParseKind { + LateAttrParsingNever = 0, + LateAttrParsingAlways = 1, + LateAttrParsingExperimentalOnly = 2 +}; + +static LateAttrParseKind getLateAttrParseKind(const Record *Attr) { + // This function basically does + // `Attr->getValueAsDef("LateParsed")->getValueAsInt("Mode")` but does + // a bunch of sanity checking to ensure that + // `LateAttrParseMode` in `Attr.td` is in sync with the `LateAttrParseKind` + // enum in this source file. + + const char *LateParsedStr = "LateParsed"; + auto *LAPK = Attr->getValueAsDef(LateParsedStr); + + // Typecheck the `LateParsed` field. + SmallVector SuperClasses; + LAPK->getDirectSuperClasses(SuperClasses); + if (SuperClasses.size() != 1) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have one super class"); + } + const char *LateAttrParseKindStr = "LateAttrParseKind"; + if (SuperClasses[0]->getName().compare(LateAttrParseKindStr) != 0) { +PrintFatalError(Attr, "Field `" + llvm::Twine(LateParsedStr) + + "`should only have type `" + + llvm::Twine(LateAttrParseKindStr) + + "` but found type `" + + SuperClasses[0]->getName() + "`"); + } + + // Get Kind and verify the enum name matches the name in `Attr.td`. + const char *KindFieldStr = "Kind"; Sirraide wrote: I personally would also group the field name constants at the top of the function so they’re all in one place, but that might also just be a me thing; it’s not that important https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)
https://github.com/Sirraide edited https://github.com/llvm/llvm-project/pull/88596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][CWG1815] Support lifetime extension of temporary created by aggregate initialization using a default member initializer (PR #87933)
https://github.com/yronglin edited https://github.com/llvm/llvm-project/pull/87933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Don't break between string literal operands of << (PR #69871)
owenca wrote: > I wish we'd made this removal an option rather than just removing it, what we > are seeing is reasonably formatted json or xml being streamed out is now > poorly written > > ```c++ > osjson << "{\n" ><<" \"name\": \"value\",\n" ><<" \"key\": \"abc\", \n" ><<" }"; > ``` > > now becomes > > ```c++ > osjson << "{\n" <<" \"name\": \"value\",\n <<" \"key\": \"abc\",\n << "}"; > ``` What version was used? I've just tried clang-format 18.1.3, and it's totally fine: ``` $ cat lessless.cpp osjson << "{\n" << " \"name\": \"value\",\n" << " \"key\": \"abc\", \n" << " }"; $ clang-format -version clang-format version 18.1.3 $ clang-format lessless.cpp | diff lessless.cpp - $ ``` https://github.com/llvm/llvm-project/pull/69871 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Introduce `SemaOpenMP` (PR #88642)
Endilll wrote: I intentionally split formatting changes into a separate commit if reviewers want to look at changes without formatting noise. If, given the volume of changes here, there is an appetite to remove `OpenMP` and `OMP` from names inside `SemaOpenMP` right in this patch, I can do that. I did that for HLSL and OpenACC, but those were an order of magnitude smaller patches that this one. https://github.com/llvm/llvm-project/pull/88642 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Introduce `SemaOpenMP` (PR #88642)
llvmbot wrote: @llvm/pr-subscribers-openmp Author: Vlad Serebrennikov (Endilll) Changes This patch moves OpenMP-related entities out of `Sema` to a newly created `SemaOpenMP` class. This is a part of the effort to split `Sema` up, and follows the recent example of CUDA, OpenACC, SYCL, HLSL. Additional context can be found in https://github.com/llvm/llvm-project/pull/82217, https://github.com/llvm/llvm-project/pull/84184, https://github.com/llvm/llvm-project/pull/87634. --- Patch is 791.89 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/88642.diff 18 Files Affected: - (modified) clang/include/clang/Parse/Parser.h (+8-7) - (modified) clang/include/clang/Sema/Sema.h (+7-1412) - (added) clang/include/clang/Sema/SemaOpenMP.h (+1447) - (modified) clang/lib/Parse/ParseDecl.cpp (+2-1) - (modified) clang/lib/Parse/ParseExpr.cpp (+3-2) - (modified) clang/lib/Parse/ParseOpenMP.cpp (+128-112) - (modified) clang/lib/Parse/ParseStmt.cpp (+2-1) - (modified) clang/lib/Sema/Sema.cpp (+14-11) - (modified) clang/lib/Sema/SemaDecl.cpp (+18-14) - (modified) clang/lib/Sema/SemaDeclCXX.cpp (+5-4) - (modified) clang/lib/Sema/SemaExpr.cpp (+31-579) - (modified) clang/lib/Sema/SemaExprMember.cpp (+5-4) - (modified) clang/lib/Sema/SemaLambda.cpp (+2-1) - (modified) clang/lib/Sema/SemaOpenMP.cpp (+2506-1827) - (modified) clang/lib/Sema/SemaStmt.cpp (+4-2) - (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+32-28) - (modified) clang/lib/Sema/SemaType.cpp (+2-1) - (modified) clang/lib/Sema/TreeTransform.h (+349-335) ``diff diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index c719218731c35b..a8791c40ab52f9 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -18,6 +18,7 @@ #include "clang/Lex/CodeCompletionHandler.h" #include "clang/Lex/Preprocessor.h" #include "clang/Sema/Sema.h" +#include "clang/Sema/SemaOpenMP.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Frontend/OpenMP/OMPContext.h" #include "llvm/Support/SaveAndRestore.h" @@ -2535,7 +2536,7 @@ class Parser : public CodeCompletionHandler { /// Returns true for declaration, false for expression. bool isForInitDeclaration() { if (getLangOpts().OpenMP) - Actions.startOpenMPLoop(); + Actions.OpenMP().startOpenMPLoop(); if (getLangOpts().CPlusPlus) return Tok.is(tok::kw_using) || isCXXSimpleDeclaration(/*AllowForRangeDecl=*/true); @@ -3394,7 +3395,7 @@ class Parser : public CodeCompletionHandler { SourceLocation Loc); /// Parse clauses for '#pragma omp [begin] declare target'. - void ParseOMPDeclareTargetClauses(Sema::DeclareTargetContextInfo ); + void ParseOMPDeclareTargetClauses(SemaOpenMP::DeclareTargetContextInfo ); /// Parse '#pragma omp end declare target'. void ParseOMPEndDeclareTargetDirective(OpenMPDirectiveKind BeginDKind, @@ -3484,7 +3485,7 @@ class Parser : public CodeCompletionHandler { /// Parses indirect clause /// \param ParseOnly true to skip the clause's semantic actions and return // false; - bool ParseOpenMPIndirectClause(Sema::DeclareTargetContextInfo , + bool ParseOpenMPIndirectClause(SemaOpenMP::DeclareTargetContextInfo , bool ParseOnly); /// Parses clause with a single expression and an additional argument /// of a kind \a Kind. @@ -3554,12 +3555,12 @@ class Parser : public CodeCompletionHandler { /// Parses a reserved locator like 'omp_all_memory'. bool ParseOpenMPReservedLocator(OpenMPClauseKind Kind, - Sema::OpenMPVarListDataTy , + SemaOpenMP::OpenMPVarListDataTy , const LangOptions ); /// Parses clauses with list. bool ParseOpenMPVarList(OpenMPDirectiveKind DKind, OpenMPClauseKind Kind, SmallVectorImpl , - Sema::OpenMPVarListDataTy ); + SemaOpenMP::OpenMPVarListDataTy ); bool ParseUnqualifiedId(CXXScopeSpec , ParsedType ObjectType, bool ObjectHadErrors, bool EnteringContext, bool AllowDestructorName, bool AllowConstructorName, @@ -3567,11 +3568,11 @@ class Parser : public CodeCompletionHandler { SourceLocation *TemplateKWLoc, UnqualifiedId ); /// Parses the mapper modifier in map, to, and from clauses. - bool parseMapperModifier(Sema::OpenMPVarListDataTy ); + bool parseMapperModifier(SemaOpenMP::OpenMPVarListDataTy ); /// Parses map-type-modifiers in map clause. /// map([ [map-type-modifier[,] [map-type-modifier[,] ...] map-type : ] list) /// where, map-type-modifier ::= always | close | mapper(mapper-identifier) - bool parseMapTypeModifiers(Sema::OpenMPVarListDataTy ); + bool parseMapTypeModifiers(SemaOpenMP::OpenMPVarListDataTy );
[clang] [clang] Fix high memory consumption during pack deduction (PR #88637)
Sirraide wrote: > I can supply the exact code that causes this issue if needed, but I would > appreciate if you frends can point me to any tools that can generate an > obfuscated minimal reproducible example. There’s `creduce` and `cvise` from what I recall, but I’m not particularly good at using them. Depending on how much work that would be, you could try doing ‘binary search’ manually (i.e. delete half the code, see if it compiles; if it does, the problem is in the other half). Seeing as it does seem to have something to do w/ template deduction in variadic templates, that might help narrow it down a bit. https://github.com/llvm/llvm-project/pull/88637 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix high memory consumption during pack deduction (PR #88637)
@@ -831,7 +831,7 @@ class PackDeductionScope { if (IsPartiallyExpanded) PackElements += NumPartialPackArgs; else if (IsExpanded) - PackElements += *FixedNumExpansions; + PackElements += FixedNumExpansions.value_or(1); Sirraide wrote: This seems fine as a fix, though ideally, it would be good to know what’s causing this so it’s easier to figure out what’s going wrong; my assumption is that we do something to the pack that causes `IsExpanded` to be true here but which wouldn’t have been true at the time `FixedNumExpansions` was (not) set. https://github.com/llvm/llvm-project/pull/88637 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix high memory consumption during pack deduction (PR #88637)
https://github.com/Endilll edited https://github.com/llvm/llvm-project/pull/88637 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][DR1815] Support lifetime extension of temporary created by aggregate initialization using a default member initializer (PR #87933)
https://github.com/yronglin updated https://github.com/llvm/llvm-project/pull/87933 >From 9fba6da7cb1ffbc7d46b69c6ac0cfd15a89c4b56 Mon Sep 17 00:00:00 2001 From: yronglin Date: Mon, 8 Apr 2024 01:38:23 +0800 Subject: [PATCH] [Clang] Support lifetime extension of temporary created by aggregate initialization using a default member initializer Signed-off-by: yronglin --- .../clang/Basic/DiagnosticSemaKinds.td| 6 --- clang/lib/Sema/SemaExpr.cpp | 16 clang/lib/Sema/SemaInit.cpp | 39 +-- .../Analysis/lifetime-extended-regions.cpp| 2 +- clang/test/CXX/drs/dr16xx.cpp | 2 - clang/test/CXX/drs/dr18xx.cpp | 5 +-- clang/test/CXX/special/class.temporary/p6.cpp | 20 ++ clang/test/SemaCXX/constexpr-default-arg.cpp | 4 +- clang/test/SemaCXX/eval-crashes.cpp | 6 +-- 9 files changed, 62 insertions(+), 38 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index a1dda2d2461c31..ba779e83d2afd4 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10010,12 +10010,6 @@ def warn_new_dangling_initializer_list : Warning< "the allocated initializer list}0 " "will be destroyed at the end of the full-expression">, InGroup; -def warn_unsupported_lifetime_extension : Warning< - "lifetime extension of " - "%select{temporary|backing array of initializer list}0 created " - "by aggregate initialization using a default member initializer " - "is not yet supported; lifetime of %select{temporary|backing array}0 " - "will end at the end of the full-expression">, InGroup; // For non-floating point, expressions of the form x == x or x != x // should result in a warning, since these always evaluate to a constant. diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 8db4fffeecfe35..b2e0f2a2a60113 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -6338,10 +6338,9 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc, Res = Immediate.TransformInitializer(Param->getInit(), /*NotCopy=*/false); }); - if (Res.isInvalid()) -return ExprError(); - Res = ConvertParamDefaultArgument(Param, Res.get(), -Res.get()->getBeginLoc()); + if (Res.isUsable()) +Res = ConvertParamDefaultArgument(Param, Res.get(), + Res.get()->getBeginLoc()); if (Res.isInvalid()) return ExprError(); Init = Res.get(); @@ -6377,7 +6376,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) { Expr *Init = nullptr; bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer(); - + bool InLifetimeExtendingContext = isInLifetimeExtendingContext(); EnterExpressionEvaluationContext EvalContext( *this, ExpressionEvaluationContext::PotentiallyEvaluated, Field); @@ -6412,11 +6411,14 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) { ImmediateCallVisitor V(getASTContext()); if (!NestedDefaultChecking) V.TraverseDecl(Field); - if (V.HasImmediateCalls) { + if (V.HasImmediateCalls || InLifetimeExtendingContext) { ExprEvalContexts.back().DelayedDefaultInitializationContext = {Loc, Field, CurContext}; ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer = NestedDefaultChecking; +// Pass down lifetime extending flag, and collect temporaries in +// CreateMaterializeTemporaryExpr when we rewrite the call argument. +keepInLifetimeExtendingContext(); EnsureImmediateInvocationInDefaultArgs Immediate(*this); ExprResult Res; @@ -6424,7 +6426,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) { Res = Immediate.TransformInitializer(Field->getInClassInitializer(), /*CXXDirectInit=*/false); }); -if (!Res.isInvalid()) +if (Res.isUsable()) Res = ConvertMemberDefaultInitExpression(Field, Res.get(), Loc); if (Res.isInvalid()) { Field->setInvalidDecl(); diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index a75e9925a43146..842412cd674d8c 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -710,6 +710,26 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field, if (VerifyOnly) return; + // Enter a lifetime extension context, then we can support lifetime + // extension of temporary created by aggregate initialization using a + // default member initializer (DR1815 https://wg21.link/CWG1815). + // + //
[clang] Fix an issue where clang++ and clangd uses enormous amounts of memory (PR #88637)
Endilll wrote: I'm not sure this is testable, but if you can think of a test, that would be nice. Can you also add a release note to `clang/ReleaseNotes.rst`? https://github.com/llvm/llvm-project/pull/88637 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][DR1815] Support lifetime extension of temporary created by aggregate initialization using a default member initializer (PR #87933)
https://github.com/yronglin edited https://github.com/llvm/llvm-project/pull/87933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Introduce `SemaCUDA` (PR #88559)
Endilll wrote: @fhahn Yeah, that's theoretically possible. I'll look into it today. https://github.com/llvm/llvm-project/pull/88559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix issue on requires expression with templated base class member function (PR #85198)
@@ -7735,7 +7735,8 @@ ExprResult Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl, } if (CXXMethodDecl *Method = dyn_cast_or_null(FDecl)) -if (Method->isImplicitObjectMemberFunction()) +if (!isa(CurContext) && jcsxky wrote: And if only checking whether `Method` is a static method, we failed on: https://github.com/llvm/llvm-project/blob/ef9446bd2d362ec90cd681ae59463d16bf671fe8/clang/test/CXX/drs/dr26xx.cpp#L225-L240 https://github.com/llvm/llvm-project/pull/85198 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Ignore deleted ctor in `bugprone-forwarding-reference-overload` (PR #88138)
MikeWeller wrote: Sorry, didn't realize there are release notes I can updated in the change itself. Will do this Monday. https://github.com/llvm/llvm-project/pull/88138 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix an issue where clang++ and clangd uses enormous amounts of memory (PR #88637)
github-actions[bot] wrote: ⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo. Please turn off [Keep my email addresses private](https://github.com/settings/emails) setting in your account. See [LLVM Discourse](https://discourse.llvm.org/t/hidden-emails-on-github-should-we-do-something-about-it) for more information. https://github.com/llvm/llvm-project/pull/88637 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] New clang-format-indent-mode for Emacs (PR #78904)
amygrinn wrote: ping https://github.com/llvm/llvm-project/pull/78904 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] check `std::string_view` and custom string-like classes in `readability-string-compare` (PR #88636)
llvmbot wrote: @llvm/pr-subscribers-clang-tools-extra Author: Vadim D. (vvd170501) Changes This PR aims to expand the list of classes that are considered to be "strings" by `readability-string-compare` check. 1. Currently only `std::string;:compare` is checked, but `std::string_view` has a similar `compare` method. This PR enables checking of `std::string_view::compare` by default. 2. Some codebases use custom string-like classes that have public interfaces similar to `std::string` or `std::string_view`. Example: [TStringBase](https://github.com/yandex/yatool/blob/main/util/generic/strbase.h#L38), A new option, `readability-string-compare.StringClassNames`, is added to allow specifying a custom list of string-like classes. Related to, but does not solve #28396 (only adds support for custom string-like classes, not custom functions) --- Full diff: https://github.com/llvm/llvm-project/pull/88636.diff 7 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp (+51-23) - (modified) clang-tools-extra/clang-tidy/readability/StringCompareCheck.h (+7-2) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) - (modified) clang-tools-extra/docs/clang-tidy/checks/readability/string-compare.rst (+31-2) - (modified) clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string (+10) - (added) clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp (+38) - (modified) clang-tools-extra/test/clang-tidy/checkers/readability/string-compare.cpp (+23) ``diff diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp index 3b5d89c8c64719..905e5b156ef864 100644 --- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp @@ -7,12 +7,16 @@ //===--===// #include "StringCompareCheck.h" -#include "../utils/FixItHintUtils.h" +#include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Tooling/FixIt.h" +#include "llvm/ADT/StringRef.h" +#include using namespace clang::ast_matchers; +namespace optutils = clang::tidy::utils::options; namespace clang::tidy::readability { @@ -20,29 +24,53 @@ static const StringRef CompareMessage = "do not use 'compare' to test equality " "of strings; use the string equality " "operator instead"; +static const std::vector StringClasses = { +"::std::basic_string", "::std::basic_string_view"}; + +StringCompareCheck::StringCompareCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + StringLikeClasses( + optutils::parseStringList(Options.get("StringLikeClasses", ""))) {} + void StringCompareCheck::registerMatchers(MatchFinder *Finder) { - const auto StrCompare = cxxMemberCallExpr( - callee(cxxMethodDecl(hasName("compare"), - ofClass(classTemplateSpecializationDecl( - hasName("::std::basic_string"), - hasArgument(0, expr().bind("str2")), argumentCountIs(1), - callee(memberExpr().bind("str1"))); - - // First and second case: cast str.compare(str) to boolean. - Finder->addMatcher( - traverse(TK_AsIs, - implicitCastExpr(hasImplicitDestinationType(booleanType()), -has(StrCompare)) - .bind("match1")), - this); - - // Third and fourth case: str.compare(str) == 0 and str.compare(str) != 0. - Finder->addMatcher( - binaryOperator(hasAnyOperatorName("==", "!="), - hasOperands(StrCompare.bind("compare"), - integerLiteral(equals(0)).bind("zero"))) - .bind("match2"), - this); + const auto RegisterForClasses = [&, this](const auto ) { +const auto StrCompare = cxxMemberCallExpr( +callee(cxxMethodDecl(hasName("compare"), ofClass(StringClassMatcher))), +hasArgument(0, expr().bind("str2")), argumentCountIs(1), +callee(memberExpr().bind("str1"))); + +// First and second case: cast str.compare(str) to boolean. +Finder->addMatcher( +traverse(TK_AsIs, + implicitCastExpr(hasImplicitDestinationType(booleanType()), + has(StrCompare)) + .bind("match1")), +this); + +// Third and fourth case: str.compare(str) == 0 +// and str.compare(str) != 0. +Finder->addMatcher( +binaryOperator(hasAnyOperatorName("==", "!="), + hasOperands(StrCompare.bind("compare"), +
[clang-tools-extra] [clang-tidy] check `std::string_view` and custom string-like classes in `readability-string-compare` (PR #88636)
llvmbot wrote: @llvm/pr-subscribers-clang-tidy Author: Vadim D. (vvd170501) Changes This PR aims to expand the list of classes that are considered to be "strings" by `readability-string-compare` check. 1. Currently only `std::string;:compare` is checked, but `std::string_view` has a similar `compare` method. This PR enables checking of `std::string_view::compare` by default. 2. Some codebases use custom string-like classes that have public interfaces similar to `std::string` or `std::string_view`. Example: [TStringBase](https://github.com/yandex/yatool/blob/main/util/generic/strbase.h#L38), A new option, `readability-string-compare.StringClassNames`, is added to allow specifying a custom list of string-like classes. Related to, but does not solve #28396 (only adds support for custom string-like classes, not custom functions) --- Full diff: https://github.com/llvm/llvm-project/pull/88636.diff 7 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp (+51-23) - (modified) clang-tools-extra/clang-tidy/readability/StringCompareCheck.h (+7-2) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) - (modified) clang-tools-extra/docs/clang-tidy/checks/readability/string-compare.rst (+31-2) - (modified) clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string (+10) - (added) clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp (+38) - (modified) clang-tools-extra/test/clang-tidy/checkers/readability/string-compare.cpp (+23) ``diff diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp index 3b5d89c8c64719..905e5b156ef864 100644 --- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp @@ -7,12 +7,16 @@ //===--===// #include "StringCompareCheck.h" -#include "../utils/FixItHintUtils.h" +#include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Tooling/FixIt.h" +#include "llvm/ADT/StringRef.h" +#include using namespace clang::ast_matchers; +namespace optutils = clang::tidy::utils::options; namespace clang::tidy::readability { @@ -20,29 +24,53 @@ static const StringRef CompareMessage = "do not use 'compare' to test equality " "of strings; use the string equality " "operator instead"; +static const std::vector StringClasses = { +"::std::basic_string", "::std::basic_string_view"}; + +StringCompareCheck::StringCompareCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + StringLikeClasses( + optutils::parseStringList(Options.get("StringLikeClasses", ""))) {} + void StringCompareCheck::registerMatchers(MatchFinder *Finder) { - const auto StrCompare = cxxMemberCallExpr( - callee(cxxMethodDecl(hasName("compare"), - ofClass(classTemplateSpecializationDecl( - hasName("::std::basic_string"), - hasArgument(0, expr().bind("str2")), argumentCountIs(1), - callee(memberExpr().bind("str1"))); - - // First and second case: cast str.compare(str) to boolean. - Finder->addMatcher( - traverse(TK_AsIs, - implicitCastExpr(hasImplicitDestinationType(booleanType()), -has(StrCompare)) - .bind("match1")), - this); - - // Third and fourth case: str.compare(str) == 0 and str.compare(str) != 0. - Finder->addMatcher( - binaryOperator(hasAnyOperatorName("==", "!="), - hasOperands(StrCompare.bind("compare"), - integerLiteral(equals(0)).bind("zero"))) - .bind("match2"), - this); + const auto RegisterForClasses = [&, this](const auto ) { +const auto StrCompare = cxxMemberCallExpr( +callee(cxxMethodDecl(hasName("compare"), ofClass(StringClassMatcher))), +hasArgument(0, expr().bind("str2")), argumentCountIs(1), +callee(memberExpr().bind("str1"))); + +// First and second case: cast str.compare(str) to boolean. +Finder->addMatcher( +traverse(TK_AsIs, + implicitCastExpr(hasImplicitDestinationType(booleanType()), + has(StrCompare)) + .bind("match1")), +this); + +// Third and fourth case: str.compare(str) == 0 +// and str.compare(str) != 0. +Finder->addMatcher( +binaryOperator(hasAnyOperatorName("==", "!="), + hasOperands(StrCompare.bind("compare"), +
[clang-tools-extra] [clang-tidy] check `std::string_view` and custom string-like classes in `readability-string-compare` (PR #88636)
https://github.com/vvd170501 ready_for_review https://github.com/llvm/llvm-project/pull/88636 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] check `std::string_view` and custom string-like classes in `readability-string-compare` (PR #88636)
@@ -7,42 +7,70 @@ //===--===// #include "StringCompareCheck.h" -#include "../utils/FixItHintUtils.h" +#include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Tooling/FixIt.h" +#include "llvm/ADT/StringRef.h" +#include using namespace clang::ast_matchers; +namespace optutils = clang::tidy::utils::options; namespace clang::tidy::readability { static const StringRef CompareMessage = "do not use 'compare' to test equality " "of strings; use the string equality " "operator instead"; +static const std::vector StringClasses = { +"::std::basic_string", "::std::basic_string_view"}; + +StringCompareCheck::StringCompareCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + StringLikeClasses( + optutils::parseStringList(Options.get("StringLikeClasses", ""))) {} + void StringCompareCheck::registerMatchers(MatchFinder *Finder) { - const auto StrCompare = cxxMemberCallExpr( - callee(cxxMethodDecl(hasName("compare"), - ofClass(classTemplateSpecializationDecl( - hasName("::std::basic_string"), - hasArgument(0, expr().bind("str2")), argumentCountIs(1), - callee(memberExpr().bind("str1"))); - - // First and second case: cast str.compare(str) to boolean. - Finder->addMatcher( - traverse(TK_AsIs, - implicitCastExpr(hasImplicitDestinationType(booleanType()), -has(StrCompare)) - .bind("match1")), - this); - - // Third and fourth case: str.compare(str) == 0 and str.compare(str) != 0. - Finder->addMatcher( - binaryOperator(hasAnyOperatorName("==", "!="), - hasOperands(StrCompare.bind("compare"), - integerLiteral(equals(0)).bind("zero"))) - .bind("match2"), - this); + const auto RegisterForClasses = [&, this](const auto ) { vvd170501 wrote: Type of `StringClassMatcher` dependinds on whether the check is used with or without `StringLikeClasses`, so a template lambda is needed. It's possible to use `anyOf` and `cxxRecordDecl` in both cases and remove the lambda, but this will probably slow the check down in default case. https://github.com/llvm/llvm-project/pull/88636 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] check `std::string_view` and custom string-like classes in `readability-string-compare` (PR #88636)
https://github.com/vvd170501 updated https://github.com/llvm/llvm-project/pull/88636 >From 0db24a6806e9429b5e7b9cd9d0777315b3e6d87e Mon Sep 17 00:00:00 2001 From: Vadim Dudkin Date: Sat, 13 Apr 2024 23:36:12 +0300 Subject: [PATCH] readability-string-compare: check std::string_view, allow custom string-like classes --- .../readability/StringCompareCheck.cpp| 74 +-- .../readability/StringCompareCheck.h | 9 ++- clang-tools-extra/docs/ReleaseNotes.rst | 5 ++ .../checks/readability/string-compare.rst | 33 - .../clang-tidy/checkers/Inputs/Headers/string | 10 +++ .../string-compare-custom-string-classes.cpp | 38 ++ .../checkers/readability/string-compare.cpp | 23 ++ 7 files changed, 165 insertions(+), 27 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp index 3b5d89c8c64719..905e5b156ef864 100644 --- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp @@ -7,12 +7,16 @@ //===--===// #include "StringCompareCheck.h" -#include "../utils/FixItHintUtils.h" +#include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Tooling/FixIt.h" +#include "llvm/ADT/StringRef.h" +#include using namespace clang::ast_matchers; +namespace optutils = clang::tidy::utils::options; namespace clang::tidy::readability { @@ -20,29 +24,53 @@ static const StringRef CompareMessage = "do not use 'compare' to test equality " "of strings; use the string equality " "operator instead"; +static const std::vector StringClasses = { +"::std::basic_string", "::std::basic_string_view"}; + +StringCompareCheck::StringCompareCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + StringLikeClasses( + optutils::parseStringList(Options.get("StringLikeClasses", ""))) {} + void StringCompareCheck::registerMatchers(MatchFinder *Finder) { - const auto StrCompare = cxxMemberCallExpr( - callee(cxxMethodDecl(hasName("compare"), - ofClass(classTemplateSpecializationDecl( - hasName("::std::basic_string"), - hasArgument(0, expr().bind("str2")), argumentCountIs(1), - callee(memberExpr().bind("str1"))); - - // First and second case: cast str.compare(str) to boolean. - Finder->addMatcher( - traverse(TK_AsIs, - implicitCastExpr(hasImplicitDestinationType(booleanType()), -has(StrCompare)) - .bind("match1")), - this); - - // Third and fourth case: str.compare(str) == 0 and str.compare(str) != 0. - Finder->addMatcher( - binaryOperator(hasAnyOperatorName("==", "!="), - hasOperands(StrCompare.bind("compare"), - integerLiteral(equals(0)).bind("zero"))) - .bind("match2"), - this); + const auto RegisterForClasses = [&, this](const auto ) { +const auto StrCompare = cxxMemberCallExpr( +callee(cxxMethodDecl(hasName("compare"), ofClass(StringClassMatcher))), +hasArgument(0, expr().bind("str2")), argumentCountIs(1), +callee(memberExpr().bind("str1"))); + +// First and second case: cast str.compare(str) to boolean. +Finder->addMatcher( +traverse(TK_AsIs, + implicitCastExpr(hasImplicitDestinationType(booleanType()), + has(StrCompare)) + .bind("match1")), +this); + +// Third and fourth case: str.compare(str) == 0 +// and str.compare(str) != 0. +Finder->addMatcher( +binaryOperator(hasAnyOperatorName("==", "!="), + hasOperands(StrCompare.bind("compare"), + integerLiteral(equals(0)).bind("zero"))) +.bind("match2"), +this); + }; + if (StringLikeClasses.empty()) { +RegisterForClasses( +classTemplateSpecializationDecl(hasAnyName(StringClasses))); + } else { +// StringLikeClasses may or may not be templates, so we need to match both +// template and non-template classes. +std::vector PossiblyTemplateClasses = StringClasses; +PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(), + StringLikeClasses.begin(), + StringLikeClasses.end()); +RegisterForClasses(anyOf( +
[clang-tools-extra] [clang-tidy] check `std::string_view` and custom string-like classes in `readability-string-compare` (PR #88636)
https://github.com/vvd170501 updated https://github.com/llvm/llvm-project/pull/88636 >From b63dd11ea87cd504c8972de6ed29330d6702abca Mon Sep 17 00:00:00 2001 From: Vadim Dudkin Date: Sat, 13 Apr 2024 23:36:12 +0300 Subject: [PATCH] readability-string-compare: check std::string_view, allow custom string-like classes --- .../readability/StringCompareCheck.cpp| 74 +-- .../readability/StringCompareCheck.h | 9 ++- clang-tools-extra/docs/ReleaseNotes.rst | 5 ++ .../checks/readability/string-compare.rst | 33 - .../clang-tidy/checkers/Inputs/Headers/string | 10 +++ .../string-compare-custom-string-classes.cpp | 36 + .../checkers/readability/string-compare.cpp | 23 ++ 7 files changed, 163 insertions(+), 27 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp index 3b5d89c8c64719..905e5b156ef864 100644 --- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp @@ -7,12 +7,16 @@ //===--===// #include "StringCompareCheck.h" -#include "../utils/FixItHintUtils.h" +#include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Tooling/FixIt.h" +#include "llvm/ADT/StringRef.h" +#include using namespace clang::ast_matchers; +namespace optutils = clang::tidy::utils::options; namespace clang::tidy::readability { @@ -20,29 +24,53 @@ static const StringRef CompareMessage = "do not use 'compare' to test equality " "of strings; use the string equality " "operator instead"; +static const std::vector StringClasses = { +"::std::basic_string", "::std::basic_string_view"}; + +StringCompareCheck::StringCompareCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + StringLikeClasses( + optutils::parseStringList(Options.get("StringLikeClasses", ""))) {} + void StringCompareCheck::registerMatchers(MatchFinder *Finder) { - const auto StrCompare = cxxMemberCallExpr( - callee(cxxMethodDecl(hasName("compare"), - ofClass(classTemplateSpecializationDecl( - hasName("::std::basic_string"), - hasArgument(0, expr().bind("str2")), argumentCountIs(1), - callee(memberExpr().bind("str1"))); - - // First and second case: cast str.compare(str) to boolean. - Finder->addMatcher( - traverse(TK_AsIs, - implicitCastExpr(hasImplicitDestinationType(booleanType()), -has(StrCompare)) - .bind("match1")), - this); - - // Third and fourth case: str.compare(str) == 0 and str.compare(str) != 0. - Finder->addMatcher( - binaryOperator(hasAnyOperatorName("==", "!="), - hasOperands(StrCompare.bind("compare"), - integerLiteral(equals(0)).bind("zero"))) - .bind("match2"), - this); + const auto RegisterForClasses = [&, this](const auto ) { +const auto StrCompare = cxxMemberCallExpr( +callee(cxxMethodDecl(hasName("compare"), ofClass(StringClassMatcher))), +hasArgument(0, expr().bind("str2")), argumentCountIs(1), +callee(memberExpr().bind("str1"))); + +// First and second case: cast str.compare(str) to boolean. +Finder->addMatcher( +traverse(TK_AsIs, + implicitCastExpr(hasImplicitDestinationType(booleanType()), + has(StrCompare)) + .bind("match1")), +this); + +// Third and fourth case: str.compare(str) == 0 +// and str.compare(str) != 0. +Finder->addMatcher( +binaryOperator(hasAnyOperatorName("==", "!="), + hasOperands(StrCompare.bind("compare"), + integerLiteral(equals(0)).bind("zero"))) +.bind("match2"), +this); + }; + if (StringLikeClasses.empty()) { +RegisterForClasses( +classTemplateSpecializationDecl(hasAnyName(StringClasses))); + } else { +// StringLikeClasses may or may not be templates, so we need to match both +// template and non-template classes. +std::vector PossiblyTemplateClasses = StringClasses; +PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(), + StringLikeClasses.begin(), + StringLikeClasses.end()); +RegisterForClasses(anyOf( +
[clang-tools-extra] [clang-tidy] check `std::string_view` and custom string-like classes in `readability-string-compare` (PR #88636)
https://github.com/vvd170501 updated https://github.com/llvm/llvm-project/pull/88636 >From 91eccdd291fa4f0753aa8e9970fa146516823e22 Mon Sep 17 00:00:00 2001 From: Vadim Dudkin Date: Sat, 13 Apr 2024 23:36:12 +0300 Subject: [PATCH] readability-string-compare: check std::string_view, allow custom string-like classes --- .../readability/StringCompareCheck.cpp| 73 +-- .../readability/StringCompareCheck.h | 9 ++- clang-tools-extra/docs/ReleaseNotes.rst | 5 ++ .../checks/readability/string-compare.rst | 33 - .../clang-tidy/checkers/Inputs/Headers/string | 10 +++ .../string-compare-custom-string-classes.cpp | 36 + .../checkers/readability/string-compare.cpp | 23 ++ 7 files changed, 162 insertions(+), 27 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp index 3b5d89c8c64719..20218975ec121b 100644 --- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp @@ -7,12 +7,16 @@ //===--===// #include "StringCompareCheck.h" -#include "../utils/FixItHintUtils.h" +#include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Tooling/FixIt.h" +#include "llvm/ADT/StringRef.h" +#include using namespace clang::ast_matchers; +namespace optutils = clang::tidy::utils::options; namespace clang::tidy::readability { @@ -20,29 +24,52 @@ static const StringRef CompareMessage = "do not use 'compare' to test equality " "of strings; use the string equality " "operator instead"; +static const std::vector StringClasses = { +"::std::basic_string", "::std::basic_string_view"}; + +StringCompareCheck::StringCompareCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + StringLikeClasses( + optutils::parseStringList(Options.get("StringLikeClasses", ""))) {} + void StringCompareCheck::registerMatchers(MatchFinder *Finder) { - const auto StrCompare = cxxMemberCallExpr( - callee(cxxMethodDecl(hasName("compare"), - ofClass(classTemplateSpecializationDecl( - hasName("::std::basic_string"), - hasArgument(0, expr().bind("str2")), argumentCountIs(1), - callee(memberExpr().bind("str1"))); - - // First and second case: cast str.compare(str) to boolean. - Finder->addMatcher( - traverse(TK_AsIs, - implicitCastExpr(hasImplicitDestinationType(booleanType()), -has(StrCompare)) - .bind("match1")), - this); - - // Third and fourth case: str.compare(str) == 0 and str.compare(str) != 0. - Finder->addMatcher( - binaryOperator(hasAnyOperatorName("==", "!="), - hasOperands(StrCompare.bind("compare"), - integerLiteral(equals(0)).bind("zero"))) - .bind("match2"), - this); + const auto RegisterForClasses = [&, this](const auto ) { +const auto StrCompare = cxxMemberCallExpr( +callee(cxxMethodDecl(hasName("compare"), ofClass(StringClassMatcher))), +hasArgument(0, expr().bind("str2")), argumentCountIs(1), +callee(memberExpr().bind("str1"))); + +// First and second case: cast str.compare(str) to boolean. +Finder->addMatcher( +traverse(TK_AsIs, + implicitCastExpr(hasImplicitDestinationType(booleanType()), + has(StrCompare)) + .bind("match1")), +this); + +// Third and fourth case: str.compare(str) == 0 +// and str.compare(str) != 0. +Finder->addMatcher( +binaryOperator(hasAnyOperatorName("==", "!="), + hasOperands(StrCompare.bind("compare"), + integerLiteral(equals(0)).bind("zero"))) +.bind("match2"), +this); + }; + if (StringLikeClasses.empty()) { +RegisterForClasses( +classTemplateSpecializationDecl(hasAnyName(StringClasses))); + } else { +// StringLikeClasses may or may not be templates, so we need to match both. +std::vector PossiblyTemplateClasses = StringClasses; +PossiblyTemplateClasses.insert(PossiblyTemplateClasses.end(), + StringLikeClasses.begin(), + StringLikeClasses.end()); +RegisterForClasses(anyOf( +classTemplateSpecializationDecl(hasAnyName(PossiblyTemplateClasses)), +
[clang] Fix an issue where clang++ and clangd uses enormous amounts of memory (PR #88637)
https://github.com/term-est edited https://github.com/llvm/llvm-project/pull/88637 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add ``ignoringParenImpCasts`` in ``hasAnyArgument`` fix#75754 (PR #87268)
https://github.com/komalverma04 updated https://github.com/llvm/llvm-project/pull/87268 >From 9b5781108081565e4009c3809eab623387655f1c Mon Sep 17 00:00:00 2001 From: komalverma04 Date: Mon, 1 Apr 2024 22:43:10 +0530 Subject: [PATCH 1/7] [clang-tidy] Add ignoringParenImpCasts in hasAnyArgument --- .../bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp| 2 +- .../clang-tidy/bugprone/MisplacedWideningCastCheck.cpp| 2 +- .../bugprone/MultipleNewInOneExpressionCheck.cpp | 8 .../clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp | 2 +- .../bugprone/StringLiteralWithEmbeddedNulCheck.cpp| 2 +- .../bugprone/SuspiciousStringviewDataUsageCheck.cpp | 2 +- .../clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp| 4 ++-- .../clang-tidy/modernize/UseEmplaceCheck.cpp | 6 +++--- .../performance/InefficientStringConcatenationCheck.cpp | 4 ++-- 9 files changed, 16 insertions(+), 16 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp index 40e4ab6c8b12af..415183d5c57ba7 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp @@ -24,7 +24,7 @@ void MisplacedOperatorInStrlenInAllocCheck::registerMatchers( const auto BadUse = callExpr(callee(StrLenFunc), - hasAnyArgument(ignoringImpCasts( + hasAnyArgument(ignoringParenImpCasts( binaryOperator( hasOperatorName("+"), hasRHS(ignoringParenImpCasts(integerLiteral(equals(1) diff --git a/clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp index a1f92aae55448c..b62829a3776572 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp @@ -42,7 +42,7 @@ void MisplacedWideningCastCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher(varDecl(hasInitializer(Cast)), this); Finder->addMatcher(returnStmt(hasReturnValue(Cast)), this); - Finder->addMatcher(callExpr(hasAnyArgument(Cast)), this); + Finder->addMatcher(callExpr(hasAnyArgument(ignoringParenImpCasts(Cast))), this); Finder->addMatcher(binaryOperator(hasOperatorName("="), hasRHS(Cast)), this); Finder->addMatcher( binaryOperator(isComparisonOperator(), hasEitherOperand(Cast)), this); diff --git a/clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp index 41191a3cfed23a..b8dbea600fd368 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp @@ -96,17 +96,17 @@ void MultipleNewInOneExpressionCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( callExpr( hasAnyArgument( - expr(HasNewExpr1).bind("arg1")), + ignoringParenImpCasts(expr(HasNewExpr1).bind("arg1"))), hasAnyArgument( - expr(HasNewExpr2, unless(equalsBoundNode("arg1"))).bind("arg2")), + ignoringParenImpCasts(expr(HasNewExpr2, unless(equalsBoundNode("arg1"))).bind("arg2"))), hasAncestor(BadAllocCatchingTryBlock)), this); Finder->addMatcher( cxxConstructExpr( hasAnyArgument( - expr(HasNewExpr1).bind("arg1")), + ignoringParenImpCasts(expr(HasNewExpr1).bind("arg1"))), hasAnyArgument( - expr(HasNewExpr2, unless(equalsBoundNode("arg1"))).bind("arg2")), + ignoringParenImpCasts(expr(HasNewExpr2, unless(equalsBoundNode("arg1"))).bind("arg2"))), unless(isListInitialization()), hasAncestor(BadAllocCatchingTryBlock)), this); diff --git a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp index 977241e91b9a93..d322f2488f8082 100644 --- a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp @@ -621,7 +621,7 @@ void NotNullTerminatedResultCheck::registerMatchers(MatchFinder *Finder) { auto MallocLengthExpr = allOf( callee(functionDecl( hasAnyName("::alloca", "::calloc", "malloc", "realloc"))), - hasAnyArgument(allOf(unless(SizeExpr), expr().bind(DestMallocExprName; + hasAnyArgument(ignoringParenImpCasts(allOf(unless(SizeExpr), expr().bind(DestMallocExprName); // - Example: (char *)malloc(length); auto DestMalloc = anyOf(callExpr(MallocLengthExpr), diff --git
[clang] Fix an issue where clang++ and clangd uses enormous amounts of memory (PR #88637)
https://github.com/term-est edited https://github.com/llvm/llvm-project/pull/88637 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix an issue where clang++ and clang uses enormous amounts of memory (PR #88637)
llvmbot wrote: @llvm/pr-subscribers-clang Author: None (term-est) Changes Henlo frens! We folks at Armelsan Defense has been using LLVM for quite some time. Recently we encountered an issue with clangd where it tries to allocate 137 gigs of memory in one of our template heavy codebases. We recompiled the trunk with sanitizers, and got this - ``` I[20:24:45.715] -- reply(1) LLVM ERROR: SmallVector capacity unable to grow. Requested capacity (4294963200) is larger than maximum value for size type (4294967295) PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace. ``` So this is not a leak. Notice that requested capacity is 4294963200, which is quite near to i32 max. Further tests has showed that ```cpp /// Prepare to directly deduce arguments of the parameter with index \p Index. PackDeductionScope(Sema S, TemplateParameterList *TemplateParams, SmallVectorImplDeducedTemplateArgument Deduced, TemplateDeductionInfo Info, unsigned Index) : S(S), TemplateParams(TemplateParams), Deduced(Deduced), Info(Info) { addPack(Index); finishConstruction(1); } private: void addPack(unsigned Index) { // Save the deduced template argument for the parameter pack expanded // by this pack expansion, then clear out the deduction. DeducedFromEarlierParameter = !Deduced[Index].isNull(); DeducedPack Pack(Index); Pack.Saved = Deduced[Index]; Deduced[Index] = TemplateArgument(); // FIXME: What if we encounter multiple packs with different numbers of // pre-expanded expansions? (This should already have been diagnosed // during substitution.) if (std::optionalunsigned ExpandedPackExpansions = getExpandedPackSize(TemplateParams-getParam(Index))) FixedNumExpansions = ExpandedPackExpansions; Packs.push_back(Pack); [clangd-stacktrace.txt](https://github.com/llvm/llvm-project/files/14968656/clangd-stacktrace.txt) } ``` `addPack` might not initialize the `std::optionalunsigned FixedNumExpansions` given that `getExpandedPackSize` returns a `nullopt`, which causes the access to `FixedNumExpansions` via the `operator*` to be Undefined. `PackElements` is eventually used in `SmallVector::grow_pod`, and vector tries to allocate 137 gigs. Attached, you can find the full stacktrace. [clangd-stacktrace.txt](https://github.com/llvm/llvm-project/files/14968658/clangd-stacktrace.txt) I can supply the exact code that causes this issue if needed, but I would appreciate if you frends can point me to any tools that can generate an obfuscated minimal reproducible example. Although this was discovered in clangd, it also appears to affect clang++ as well. ![image](https://github.com/llvm/llvm-project/assets/62337595/74b907c6-4511-40cf-97cf-f6c096dff05a) ![image](https://github.com/llvm/llvm-project/assets/62337595/b905f1e0-6f41-4987-8b57-8891efe02b06) After this change, both seems to work just fine with clangd using only 300mb and clang++ compiling the codebase successfully and correctly. Thank you for your amazing work and thanks for the review~ --- Full diff: https://github.com/llvm/llvm-project/pull/88637.diff 1 Files Affected: - (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+1-1) ``diff diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp index 0b6375001f5326..1679852cdb386b 100644 --- a/clang/lib/Sema/SemaTemplateDeduction.cpp +++ b/clang/lib/Sema/SemaTemplateDeduction.cpp @@ -831,7 +831,7 @@ class PackDeductionScope { if (IsPartiallyExpanded) PackElements += NumPartialPackArgs; else if (IsExpanded) - PackElements += *FixedNumExpansions; + PackElements += FixedNumExpansions.value_or(1); for (auto : Packs) { if (Info.PendingDeducedPacks.size() > Pack.Index) `` https://github.com/llvm/llvm-project/pull/88637 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix an issue where clang++ and clang uses enormous amounts of memory (PR #88637)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/88637 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix an issue where clang++ and clang uses enormous amounts of memory (PR #88637)
https://github.com/term-est created https://github.com/llvm/llvm-project/pull/88637 Henlo frens! We folks at Armelsan Defense has been using LLVM for quite some time. Recently we encountered an issue with clangd where it tries to allocate 137 gigs of memory in one of our template heavy codebases. We recompiled the trunk with sanitizers, and got this -> ``` I[20:24:45.715] <-- reply(1) LLVM ERROR: SmallVector capacity unable to grow. Requested capacity (4294963200) is larger than maximum value for size type (4294967295) PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace. ``` So this is not a leak. Notice that requested capacity is 4294963200, which is quite near to i32 max. Further tests has showed that ```cpp /// Prepare to directly deduce arguments of the parameter with index \p Index. PackDeductionScope(Sema , TemplateParameterList *TemplateParams, SmallVectorImpl , TemplateDeductionInfo , unsigned Index) : S(S), TemplateParams(TemplateParams), Deduced(Deduced), Info(Info) { addPack(Index); finishConstruction(1); } private: void addPack(unsigned Index) { // Save the deduced template argument for the parameter pack expanded // by this pack expansion, then clear out the deduction. DeducedFromEarlierParameter = !Deduced[Index].isNull(); DeducedPack Pack(Index); Pack.Saved = Deduced[Index]; Deduced[Index] = TemplateArgument(); // FIXME: What if we encounter multiple packs with different numbers of // pre-expanded expansions? (This should already have been diagnosed // during substitution.) if (std::optional ExpandedPackExpansions = getExpandedPackSize(TemplateParams->getParam(Index))) FixedNumExpansions = ExpandedPackExpansions; Packs.push_back(Pack); [clangd-stacktrace.txt](https://github.com/llvm/llvm-project/files/14968656/clangd-stacktrace.txt) } ``` `addPack` might not initialize the `std::optional FixedNumExpansions` given that `getExpandedPackSize` returns a `nullopt`, which causes the access to `FixedNumExpansions` via the `operator*` to be Undefined. `PackElements` is eventually used in `SmallVector::grow_pod`, and vector tries to allocate 137 gigs. Attached, you can find the full stacktrace. [clangd-stacktrace.txt](https://github.com/llvm/llvm-project/files/14968658/clangd-stacktrace.txt) I can supply the exact code that causes this issue if needed, but I would appreciate if you frends can point me to any tools that can generate an obfuscated minimal reproducible example. Although this was discovered in clangd, it also appears to affect clang++ as well. ![image](https://github.com/llvm/llvm-project/assets/62337595/74b907c6-4511-40cf-97cf-f6c096dff05a) ![image](https://github.com/llvm/llvm-project/assets/62337595/b905f1e0-6f41-4987-8b57-8891efe02b06) After this change, both seems to work just fine with clangd using only 300mb and clang++ compiling the codebase successfully and correctly. Thank you for your amazing work and thanks for the review~ >From 1c639842d1b5b79692c097df24757a90eeac888f Mon Sep 17 00:00:00 2001 From: term-est <62337595+term-...@users.noreply.github.com> Date: Sat, 13 Apr 2024 23:04:00 +0300 Subject: [PATCH] Fix the bug which causes the SmallVector to be not so small. PackDeductionScope::PackDeductionScope calls PackDeductionScope::addPack, which might not assign a value to PackDeductionScope::FixedNumExpansions given that getExpandedPackSize returns a nullopt. Access to an empty std::optional via the operator* is UB, and there is a case where IsExpanded is true while FixedNumberExpansions is empty. We access the empty optional, and this value is eventually used to in a call to SmallVector::reserve, which ends up trying to reserve 137 gigs of space and crashes clangd/clang++ --- clang/lib/Sema/SemaTemplateDeduction.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp index 0b6375001f5326..1679852cdb386b 100644 --- a/clang/lib/Sema/SemaTemplateDeduction.cpp +++ b/clang/lib/Sema/SemaTemplateDeduction.cpp @@ -831,7 +831,7 @@ class PackDeductionScope { if (IsPartiallyExpanded) PackElements += NumPartialPackArgs; else if (IsExpanded) - PackElements += *FixedNumExpansions; + PackElements += FixedNumExpansions.value_or(1); for (auto : Packs) { if (Info.PendingDeducedPacks.size() > Pack.Index) ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] check `std::string_view` and custom string-like classes in `readability-string-compare` (PR #88636)
https://github.com/vvd170501 edited https://github.com/llvm/llvm-project/pull/88636 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] check `std::string_view` and custom string-like classes in `readability-string-compare` (PR #88636)
https://github.com/vvd170501 created https://github.com/llvm/llvm-project/pull/88636 This PR aims to expand the list of classes that are considered to be "strings" by `readability-string-compare` check. 1. Currently `readability-string-compare` only checks `std::string;:compare`, but `std::string_view` has a similar `compare` method, which also should not be used to check equality of two strings. 2. Some codebases use custom string-like classes that have public interfaces similar to `std::string` or `std::string_view`. Example: [TStringBase](https://github.com/yandex/yatool/blob/main/util/generic/strbase.h#L38), Related to, but does not solve #28396 (only adds support for custom string-like classes, not custom functions) >From eb5279fd83ba114b8eb094099d5993d6bfb003e3 Mon Sep 17 00:00:00 2001 From: Vadim Dudkin Date: Sat, 13 Apr 2024 23:36:12 +0300 Subject: [PATCH] readability-string-compare: check std::string_view, allow custom string-like classes --- .../readability/StringCompareCheck.cpp| 75 +-- .../readability/StringCompareCheck.h | 10 ++- .../clang-tidy/checkers/Inputs/Headers/string | 10 +++ .../string-compare-custom-string-classes.cpp | 41 ++ .../checkers/readability/string-compare.cpp | 14 5 files changed, 125 insertions(+), 25 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/string-compare-custom-string-classes.cpp diff --git a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp index 3b5d89c8c64719..30bde1f8b75b61 100644 --- a/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/StringCompareCheck.cpp @@ -7,12 +7,15 @@ //===--===// #include "StringCompareCheck.h" -#include "../utils/FixItHintUtils.h" +#include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Tooling/FixIt.h" +#include "llvm/ADT/StringRef.h" using namespace clang::ast_matchers; +namespace optutils = clang::tidy::utils::options; namespace clang::tidy::readability { @@ -20,29 +23,55 @@ static const StringRef CompareMessage = "do not use 'compare' to test equality " "of strings; use the string equality " "operator instead"; +static const StringRef DefaultStringClassNames = "::std::basic_string;" + "::std::basic_string_view"; + +StringCompareCheck::StringCompareCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + StringClassNames(optutils::parseStringList( + Options.get("StringClassNames", DefaultStringClassNames))), + // Both std::string and std::string_view are templates, so this check only + // needs to match template classes by default. + // Custom `StringClassNames` may contain non-template classes, and + // it's impossible to tell them apart from templates just by name. + CheckNonTemplateClasses(Options.get("StringClassNames").has_value()) {} + void StringCompareCheck::registerMatchers(MatchFinder *Finder) { - const auto StrCompare = cxxMemberCallExpr( - callee(cxxMethodDecl(hasName("compare"), - ofClass(classTemplateSpecializationDecl( - hasName("::std::basic_string"), - hasArgument(0, expr().bind("str2")), argumentCountIs(1), - callee(memberExpr().bind("str1"))); - - // First and second case: cast str.compare(str) to boolean. - Finder->addMatcher( - traverse(TK_AsIs, - implicitCastExpr(hasImplicitDestinationType(booleanType()), -has(StrCompare)) - .bind("match1")), - this); - - // Third and fourth case: str.compare(str) == 0 and str.compare(str) != 0. - Finder->addMatcher( - binaryOperator(hasAnyOperatorName("==", "!="), - hasOperands(StrCompare.bind("compare"), - integerLiteral(equals(0)).bind("zero"))) - .bind("match2"), - this); + if (StringClassNames.empty()) { +return; + } + const auto RegisterForClasses = [&, this](const auto ) { +const auto StrCompare = cxxMemberCallExpr( +callee(cxxMethodDecl(hasName("compare"), ofClass(StringClassMatcher))), +hasArgument(0, expr().bind("str2")), argumentCountIs(1), +callee(memberExpr().bind("str1"))); + +// First and second case: cast str.compare(str) to boolean. +Finder->addMatcher( +traverse(TK_AsIs, + implicitCastExpr(hasImplicitDestinationType(booleanType()), +
[clang] [clang][docs] fix whitespace in AttrDocs.td (PR #88631)
chrisnc wrote: Ready to merge, but I'm not able to myself. https://github.com/llvm/llvm-project/pull/88631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] revert to string << string handling to previous default (PR #88490)
@@ -5598,11 +5598,45 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine , // FIXME: Breaking after newlines seems useful in general. Turn this into an // option and recognize more cases like endl etc, and break independent of // what comes after operator lessless. - if (Right.is(tok::lessless) && Right.Next && - Right.Next->is(tok::string_literal) && Left.is(tok::string_literal) && - Left.TokenText.ends_with("\\n\"")) { -return true; + switch (Style.BreakStreamOperator) { + case FormatStyle::BCOS_BetweenStrings: { +if (Right.is(tok::lessless) && Right.Next && Left.is(tok::string_literal) && +Right.Next->is(tok::string_literal)) { + return true; +} +break; + } + case FormatStyle::BCOS_BetweenNewlineStrings: { +if (Right.is(tok::lessless) && Right.Next && +Right.Next->is(tok::string_literal) && Left.is(tok::string_literal) && +Left.TokenText.ends_with("\\n\"")) { + return true; +} +break; + } + case FormatStyle::BCOS_Always: { +// Don't break after the very first << or >> +// but the Left token can be os or std::os so HazardyKnusperkeks wrote: Still don't understand the part of `os` or `std::os`. https://github.com/llvm/llvm-project/pull/88490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] revert to string << string handling to previous default (PR #88490)
@@ -2193,6 +2193,41 @@ struct FormatStyle { /// \version 3.7 bool BreakBeforeTernaryOperators; + /// Different ways to Break Between Chevrons. + enum BreakChevronOperatorStyle : int8_t { +/// Break using ColumnLimit rules. +/// \code +/// os << "a" << "b" << "\n"; +/// \endcode +BCOS_Never, +/// Break between adjacent strings. +/// \code +/// os << "a" +/// << "b" +/// << "\n"; +/// \endcode +BCOS_BetweenStrings, +/// Break between adjacent strings that end with \n. +/// \code +/// os << "a\n" +/// << "b" << "c\n" +/// << "\n"; +/// \endcode +BCOS_BetweenNewlineStrings, +/// Break between adjacent chevrons. +/// \code +/// os << "a\n" +/// << "b" +/// << "c\n" +/// << "\n"; +/// \endcode +BCOS_Always + }; + + /// Break Between Chevron Operators + /// \version 19 + BreakChevronOperatorStyle BreakChevronOperator; HazardyKnusperkeks wrote: That's good. https://github.com/llvm/llvm-project/pull/88490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] revert to string << string handling to previous default (PR #88490)
@@ -5598,10 +5598,34 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine , // FIXME: Breaking after newlines seems useful in general. Turn this into an // option and recognize more cases like endl etc, and break independent of // what comes after operator lessless. - if (Right.is(tok::lessless) && Right.Next && - Right.Next->is(tok::string_literal) && Left.is(tok::string_literal) && - Left.TokenText.ends_with("\\n\"")) { -return true; + if (Style.BreakChevronOperator == FormatStyle::BCOS_BetweenStrings) { +if (Right.is(tok::lessless) && Right.Next && Left.is(tok::string_literal) && +Right.Next->is(tok::string_literal)) { + return true; +} + } + if (Style.BreakChevronOperator == FormatStyle::BCOS_BetweenNewlineStrings) { +if (Right.is(tok::lessless) && Right.Next && +Right.Next->is(tok::string_literal) && Left.is(tok::string_literal) && +Left.TokenText.ends_with("\\n\"")) { + return true; +} + } + if (Style.BreakChevronOperator == FormatStyle::BCOS_Always) { +// can be std::os or os HazardyKnusperkeks wrote: Yeah ok, but I'd skip the `os`. https://github.com/llvm/llvm-project/pull/88490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Set up executor implicitly to account for init PTUs (PR #84758)
Stefan =?utf-8?q?Gränitz?= Message-ID: In-Reply-To: https://github.com/vgvassilev updated https://github.com/llvm/llvm-project/pull/84758 >From f28fc3aa917b24063707f99dd6545512630f48e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Sun, 10 Mar 2024 18:17:48 +0100 Subject: [PATCH 1/2] [clang-repl] Set up executor implicitly to account for init PTUs --- clang/lib/Interpreter/Interpreter.cpp | 32 +++ clang/test/Interpreter/execute.cpp| 4 ++-- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp index cf31456b6950ac..c6ca68db9b8056 100644 --- a/clang/lib/Interpreter/Interpreter.cpp +++ b/clang/lib/Interpreter/Interpreter.cpp @@ -229,12 +229,30 @@ IncrementalCompilerBuilder::CreateCudaHost() { } Interpreter::Interpreter(std::unique_ptr CI, - llvm::Error ) { - llvm::ErrorAsOutParameter EAO(); + llvm::Error ) { + llvm::ErrorAsOutParameter EAO(); auto LLVMCtx = std::make_unique(); TSCtx = std::make_unique(std::move(LLVMCtx)); - IncrParser = std::make_unique(*this, std::move(CI), - *TSCtx->getContext(), Err); + IncrParser = std::make_unique( + *this, std::move(CI), *TSCtx->getContext(), ErrOut); + if (ErrOut) +return; + + // Not all frontends support code-generation, e.g. ast-dump actions don't + if (IncrParser->getCodeGen()) { +if (llvm::Error Err = CreateExecutor()) { + ErrOut = joinErrors(std::move(ErrOut), std::move(Err)); + return; +} + +// Process the PTUs that came from initialization. For example -include will +// give us a header that's processed at initialization of the preprocessor. +for (PartialTranslationUnit : IncrParser->getPTUs()) + if (llvm::Error Err = Execute(PTU)) { +ErrOut = joinErrors(std::move(ErrOut), std::move(Err)); +return; + } + } } Interpreter::~Interpreter() { @@ -395,10 +413,16 @@ llvm::Error Interpreter::CreateExecutor() { return llvm::make_error("Operation failed. " "Execution engine exists", std::error_code()); + if (!IncrParser->getCodeGen()) +return llvm::make_error("Operation failed. " + "No code generator available", + std::error_code()); + llvm::Expected> JB = CreateJITBuilder(*getCompilerInstance()); if (!JB) return JB.takeError(); + llvm::Error Err = llvm::Error::success(); auto Executor = std::make_unique(*TSCtx, **JB, Err); if (!Err) diff --git a/clang/test/Interpreter/execute.cpp b/clang/test/Interpreter/execute.cpp index 6e73ed3927e815..534a54ed94fba2 100644 --- a/clang/test/Interpreter/execute.cpp +++ b/clang/test/Interpreter/execute.cpp @@ -7,6 +7,8 @@ // RUN: cat %s | clang-repl | FileCheck %s // RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s +// RUN: clang-repl -Xcc -include -Xcc %s | FileCheck %s +// RUN: clang-repl -Xcc -fsyntax-only -Xcc -include -Xcc %s extern "C" int printf(const char *, ...); int i = 42; auto r1 = printf("i = %d\n", i); @@ -19,5 +21,3 @@ auto r2 = printf("S[f=%f, m=0x%llx]\n", s.f, reinterpret_castFrom 0af28b70ea86ec56ab1ee83877d9021b791ba909 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Mon, 11 Mar 2024 14:10:58 +0100 Subject: [PATCH 2/2] [tmp] Add crash note --- clang/test/Interpreter/inline-virtual.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/Interpreter/inline-virtual.cpp b/clang/test/Interpreter/inline-virtual.cpp index 79ab8ed337ffea..d862b3354f61fe 100644 --- a/clang/test/Interpreter/inline-virtual.cpp +++ b/clang/test/Interpreter/inline-virtual.cpp @@ -14,7 +14,7 @@ struct A { int a; A(int a) : a(a) {} virtual ~A(); }; // PartialTranslationUnit. inline A::~A() { printf("~A(%d)\n", a); } -// Create one instance with new and delete it. +// Create one instance with new and delete it. We crash here now: A *a1 = new A(1); delete a1; // CHECK: ~A(1) ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
PiotrZSL wrote: @sopyb Check is for modernize, not performance, so: - Add entry in documentation that due to need of copy object into initialization list check may cause performance degradation, add entry that using std::ref, std::cref is recommended in such case: `b = std::max({std::ref(i), std::ref(j), std::ref(k)});` - Add option - IgnoreNonTrivialTypes - set by default to true - Add option - IgnoreTrivialTypesOfSizeAbove - set by default to 32 bytes Options should be easy to add, check other checks. If you want quickly deliver version 1.0, then just limit check to built-in types. As for copies of large types, that's more a thing for new performance check. https://github.com/llvm/llvm-project/pull/85572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang Format] Fixing erroneous statements in tests; nsc (PR #88628)
https://github.com/rymiel closed https://github.com/llvm/llvm-project/pull/88628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Introduce `SemaCUDA` (PR #88559)
fhahn wrote: Is it possible that this broke this bot failing with the error below? ``` usr/local/clang-17.0.2/bin/clang++ -DGTEST_HAS_RTTI=0 -D_CINDEX_LIB_ -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_LARGE_FILE_API -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/tools/libclang -I/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/clang/tools/libclang -I/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/clang/include -I/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/tools/clang/include -I/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/include -I/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/include -mcmodel=large -fPIC -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -DNDEBUG -std=c++17 -fPIC -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -MD -MT tools/clang/tools/libclang/CMakeFiles/libclang.dir/CIndexCodeCompletion.cpp.o -MF tools/clang/tools/libclang/CMakeFiles/libclang.dir/CIndexCodeCompletion.cpp.o.d -o tools/clang/tools/libclang/CMakeFiles/libclang.dir/CIndexCodeCompletion.cpp.o -c /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/clang/tools/libclang/CIndexCodeCompletion.cpp In file included from /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/clang/tools/libclang/CIndexCodeCompletion.cpp:30: In file included from /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/clang/include/clang/Sema/Sema.h:41: /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/clang/include/clang/Basic/Cuda.h:59:3: error: expected identifier 59 | SM_32, | ^ /usr/include/sys/mac.h:79:15: note: expanded from macro 'SM_32' 79 | #define SM_32 8 /* number of 32 bit words for markings */ | ^ ``` https://lab.llvm.org/buildbot/#/builders/214/builds/11887 https://github.com/llvm/llvm-project/pull/88559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang analysis] ExprMutationAnalyzer avoid infinite recursion for recursive forwarding reference (PR #87954)
https://github.com/PiotrZSL approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/87954 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang analysis] ExprMutationAnalyzer avoid infinite recursion for recursive forwarding reference (PR #87954)
@@ -22,8 +21,15 @@ class FunctionParmMutationAnalyzer; /// a given statement. class ExprMutationAnalyzer { public: + friend class FunctionParmMutationAnalyzer; + struct Cache { +llvm::DenseMaphttps://github.com/llvm/llvm-project/pull/87954 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang analysis] ExprMutationAnalyzer avoid infinite recursion for recursive forwarding reference (PR #87954)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/87954 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] `canReasonAbout` does not support `nonloc::LazyCompoundVal` (PR #87521)
https://github.com/steakhal requested changes to this pull request. https://github.com/llvm/llvm-project/pull/87521 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] `canReasonAbout` does not support `nonloc::LazyCompoundVal` (PR #87521)
@@ -0,0 +1,16 @@ +// RUN: %clang_analyze_cc1 %s \ steakhal wrote: I think you should put this test without the RUN lines into the `clang/test/Analysis/invalidated-iterator.cpp` to have them at one place. https://github.com/llvm/llvm-project/pull/87521 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] `canReasonAbout` does not support `nonloc::LazyCompoundVal` (PR #87521)
@@ -57,10 +57,14 @@ ProgramStateRef SimpleConstraintManager::assumeAux(ProgramStateRef State, // We cannot reason about SymSymExprs, and can only reason about some // SymIntExprs. if (!canReasonAbout(Cond)) { -// Just add the constraint to the expression without trying to simplify. SymbolRef Sym = Cond.getAsSymbol(); -assert(Sym); -return assumeSymUnsupported(State, Sym, Assumption); +if (Sym) { + // this will simplify the symbol, so only call this if we have a + // symbol. + return assumeSymUnsupported(State, Sym, Assumption); +} else { + return State; +} steakhal wrote: ```suggestion if (SymbolRef Sym = Cond.getAsSymbol()) return assumeSymUnsupported(State, Sym, Assumption); return State; ``` https://github.com/llvm/llvm-project/pull/87521 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] `canReasonAbout` does not support `nonloc::LazyCompoundVal` (PR #87521)
@@ -2836,6 +2836,10 @@ bool RangeConstraintManager::canReasonAbout(SVal X) const { return false; } + // Non-integer types are not supported. + if (X.getAs()) +return false; + steakhal wrote: My problem with this is that I think LCVs shouldn't even appear here. And even if they could, why don't we also handle `nonloc::CompoundVals` too? They are basically the same thing, except for the lazyness. So, this patch masks some fundamental problem somewhere else - probably within the iterator checker or container modeling, actually passing an LCV. But I get it, its better to not crash, sure. I'd be happy to merge this and creating a new ticket for investigating what's going on. https://github.com/llvm/llvm-project/pull/87521 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] `canReasonAbout` does not support `nonloc::LazyCompoundVal` (PR #87521)
@@ -0,0 +1,16 @@ +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=alpha.cplusplus.InvalidatedIterator \ +// RUN: -analyzer-config aggressive-binary-operation-simplification=true \ +// RUN: 2>&1 + +struct node {}; +struct prop : node {}; +struct bitvec : node { + prop operator==(bitvec) { return prop(); } + bitvec extend(); // { return *this; } steakhal wrote: ```suggestion bitvec extend(); ``` https://github.com/llvm/llvm-project/pull/87521 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] `canReasonAbout` does not support `nonloc::LazyCompoundVal` (PR #87521)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/87521 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Ignore deleted ctor in `bugprone-forwarding-reference-overload` (PR #88138)
https://github.com/PiotrZSL commented: Missing release notes entry, except that looks fine. https://github.com/llvm/llvm-project/pull/88138 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][docs] fix whitespace in AttrDocs.td (PR #88631)
https://github.com/cachemeifyoucan approved this pull request. LGTM. Thanks for fixing. https://github.com/llvm/llvm-project/pull/88631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Export fixes from check_clang_tidy.py (PR #88186)
https://github.com/PiotrZSL closed https://github.com/llvm/llvm-project/pull/88186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] fad3752 - [clang-tidy] Export fixes from check_clang_tidy.py (#88186)
Author: Edwin Vane Date: 2024-04-13T21:01:06+02:00 New Revision: fad37526a3ea7d669af621342968029085862281 URL: https://github.com/llvm/llvm-project/commit/fad37526a3ea7d669af621342968029085862281 DIFF: https://github.com/llvm/llvm-project/commit/fad37526a3ea7d669af621342968029085862281.diff LOG: [clang-tidy] Export fixes from check_clang_tidy.py (#88186) Makes it possible to export fixes from running llvm-lit on a clang-tidy test. To enable, modify the RUN invocation directly in the test with the new -export flag. llvm-lit will report the test passed and fixes can be found in the file specified to the -export flag. Added: Modified: clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/check_clang_tidy.py Removed: diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index b66be44e9f8a6f..1405fb0df1f8dd 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -100,6 +100,8 @@ Improvements to clang-tidy - Improved :program:`run-clang-tidy.py` script. Added argument `-source-filter` to filter source files from the compilation database, via a RegEx. In a similar fashion to what `-header-filter` does for header files. +- Improved :program:`check_clang_tidy.py` script. Added argument `-export-fixes` + to aid in clang-tidy and test development. New checks ^^ diff --git a/clang-tools-extra/test/clang-tidy/check_clang_tidy.py b/clang-tools-extra/test/clang-tidy/check_clang_tidy.py index 53ffca0bad8d06..6d4b466afa691a 100755 --- a/clang-tools-extra/test/clang-tidy/check_clang_tidy.py +++ b/clang-tools-extra/test/clang-tidy/check_clang_tidy.py @@ -8,25 +8,35 @@ # # ======# -r""" +""" ClangTidy Test Helper = -This script runs clang-tidy in fix mode and verify fixes, messages or both. +This script is used to simplify writing, running, and debugging tests compatible +with llvm-lit. By default it runs clang-tidy in fix mode and uses FileCheck to +verify messages and/or fixes. + +For debugging, with --export-fixes, the tool simply exports fixes to a provided +file and does not run FileCheck. -Usage: - check_clang_tidy.py [-resource-dir=] \ -[-assume-filename=] \ -[-check-suffix=] \ -[-check-suffixes=] \ -[-std=c++(98|11|14|17|20)[-or-later]] \ - \ --- [optional clang-tidy arguments] +Extra arguments, those after the first -- if any, are passed to either +clang-tidy or clang: +* Arguments between the first -- and second -- are clang-tidy arguments. + * May be only whitespace if there are no clang-tidy arguments. + * clang-tidy's --config would go here. +* Arguments after the second -- are clang arguments + +Examples + -Example: // RUN: %check_clang_tidy %s llvm-include-order %t -- -- -isystem %S/Inputs -Notes: +or + + // RUN: %check_clang_tidy %s llvm-include-order --export-fixes=fixes.yaml %t -std=c++20 + +Notes +- -std=c++(98|11|14|17|20)-or-later: This flag will cause multiple runs within the same check_clang_tidy execution. Make sure you don't have shared state across these runs. @@ -34,6 +44,7 @@ import argparse import os +import pathlib import re import subprocess import sys @@ -88,6 +99,7 @@ def __init__(self, args, extra_args): self.has_check_fixes = False self.has_check_messages = False self.has_check_notes = False +self.export_fixes = args.export_fixes self.fixes = MessagePrefix("CHECK-FIXES") self.messages = MessagePrefix("CHECK-MESSAGES") self.notes = MessagePrefix("CHECK-NOTES") @@ -181,7 +193,13 @@ def run_clang_tidy(self): [ "clang-tidy", self.temp_file_name, -"-fix", +] ++ [ +"-fix" +if self.export_fixes is None +else "--export-fixes=" + self.export_fixes +] ++ [ "--checks=-*," + self.check_name, ] + self.clang_tidy_extra_args @@ -255,12 +273,14 @@ def check_notes(self, clang_tidy_output): def run(self): self.read_input() -self.get_prefixes() +if self.export_fixes is None: +self.get_prefixes() self.prepare_test_inputs() clang_tidy_output = self.run_clang_tidy() -self.check_fixes() -self.check_messages(clang_tidy_output) -self.check_notes(clang_tidy_output) +if self.export_fixes is None: +self.check_fixes() +self.check_messages(clang_tidy_output) +self.check_notes(clang_tidy_output) def expand_std(std): @@ -284,7 +304,11 @@ def csv(string): def parse_arguments(): -parser =
[clang] [clang][docs] fix whitespace in AttrDocs.td (PR #88631)
chrisnc wrote: @cachemeifyoucan @cyndyishida for review? https://github.com/llvm/llvm-project/pull/88631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][docs] fix whitespace in AttrDocs.td (PR #88631)
https://github.com/chrisnc updated https://github.com/llvm/llvm-project/pull/88631 >From d3e993c34e9d05f149b2670502794eaf93dee89a Mon Sep 17 00:00:00 2001 From: Chris Copeland Date: Fri, 12 Apr 2024 23:42:32 -0700 Subject: [PATCH] [clang][docs] fix whitespace in AttrDocs.td --- clang/include/clang/Basic/AttrDocs.td | 32 +-- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 8687c4f57d3f83..a0bbe5861c5722 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -1604,39 +1604,39 @@ specifies availability for the current target platform, the availability attributes are ignored. Supported platforms are: ``ios`` - Apple's iOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-ios*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=ios*version*`` + Apple's iOS operating system. The minimum deployment target is specified + as part of the ``-target *arch*-apple-ios*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=ios*version*`` command-line argument. ``macos`` - Apple's macOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-macos*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=macos*version*`` - command-line argument. ``macosx`` is supported for + Apple's macOS operating system. The minimum deployment target is specified + as part of the ``-target *arch*-apple-macos*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=macos*version*`` + command-line argument. ``macosx`` is supported for backward-compatibility reasons, but it is deprecated. ``tvos`` - Apple's tvOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-tvos*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=tvos*version*`` + Apple's tvOS operating system. The minimum deployment target is specified + as part of the ``-target *arch*-apple-tvos*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=tvos*version*`` command-line argument. ``watchos`` Apple's watchOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-watchos*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=watchos*version*`` + as part of the ``-target *arch*-apple-watchos*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=watchos*version*`` command-line argument. ``visionos`` Apple's visionOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-visionos*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=visionos*version*`` + as part of the ``-target *arch*-apple-visionos*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=visionos*version*`` command-line argument. ``driverkit`` Apple's DriverKit userspace kernel extensions. The minimum deployment target - is specified as part of the ``-target *arch*-apple-driverkit*version*`` + is specified as part of the ``-target *arch*-apple-driverkit*version*`` command line argument. A declaration can typically be used even when deploying back to a platform @@ -7522,7 +7522,7 @@ means that it can e.g no longer be part of an initializer expression. /* This may print something else than "6 * 7 = 42", if there is a non-weak definition of "ANSWER" in -an object linked in */ + an object linked in */ printf("6 * 7 = %d\n", ANSWER); return 0; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][docs] fix whitespace in AttrDocs.td (PR #88631)
https://github.com/chrisnc edited https://github.com/llvm/llvm-project/pull/88631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][docs] fix whitespace in AttrDocs.td (PR #88631)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Chris Copeland (chrisnc) Changes --- Full diff: https://github.com/llvm/llvm-project/pull/88631.diff 1 Files Affected: - (modified) clang/include/clang/Basic/AttrDocs.td (+16-16) ``diff diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 8687c4f57d3f83..a0bbe5861c5722 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -1604,39 +1604,39 @@ specifies availability for the current target platform, the availability attributes are ignored. Supported platforms are: ``ios`` - Apple's iOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-ios*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=ios*version*`` + Apple's iOS operating system. The minimum deployment target is specified + as part of the ``-target *arch*-apple-ios*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=ios*version*`` command-line argument. ``macos`` - Apple's macOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-macos*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=macos*version*`` - command-line argument. ``macosx`` is supported for + Apple's macOS operating system. The minimum deployment target is specified + as part of the ``-target *arch*-apple-macos*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=macos*version*`` + command-line argument. ``macosx`` is supported for backward-compatibility reasons, but it is deprecated. ``tvos`` - Apple's tvOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-tvos*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=tvos*version*`` + Apple's tvOS operating system. The minimum deployment target is specified + as part of the ``-target *arch*-apple-tvos*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=tvos*version*`` command-line argument. ``watchos`` Apple's watchOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-watchos*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=watchos*version*`` + as part of the ``-target *arch*-apple-watchos*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=watchos*version*`` command-line argument. ``visionos`` Apple's visionOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-visionos*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=visionos*version*`` + as part of the ``-target *arch*-apple-visionos*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=visionos*version*`` command-line argument. ``driverkit`` Apple's DriverKit userspace kernel extensions. The minimum deployment target - is specified as part of the ``-target *arch*-apple-driverkit*version*`` + is specified as part of the ``-target *arch*-apple-driverkit*version*`` command line argument. A declaration can typically be used even when deploying back to a platform @@ -7522,7 +7522,7 @@ means that it can e.g no longer be part of an initializer expression. /* This may print something else than "6 * 7 = 42", if there is a non-weak definition of "ANSWER" in -an object linked in */ + an object linked in */ printf("6 * 7 = %d\n", ANSWER); return 0; `` https://github.com/llvm/llvm-project/pull/88631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][docs] fix whitespace in AttrDocs.td (PR #88631)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/88631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][docs] fix whitespace in AttrDocs.td (PR #88631)
https://github.com/chrisnc created https://github.com/llvm/llvm-project/pull/88631 None >From 40d774ab8c598f0dfb76dcd087f1af17c7fdd01d Mon Sep 17 00:00:00 2001 From: Chris Copeland Date: Fri, 12 Apr 2024 23:42:32 -0700 Subject: [PATCH] [clang][docs] fix whitespace in AttrDocs.td --- clang/include/clang/Basic/AttrDocs.td | 32 +-- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 8687c4f57d3f83..a0bbe5861c5722 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -1604,39 +1604,39 @@ specifies availability for the current target platform, the availability attributes are ignored. Supported platforms are: ``ios`` - Apple's iOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-ios*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=ios*version*`` + Apple's iOS operating system. The minimum deployment target is specified + as part of the ``-target *arch*-apple-ios*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=ios*version*`` command-line argument. ``macos`` - Apple's macOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-macos*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=macos*version*`` - command-line argument. ``macosx`` is supported for + Apple's macOS operating system. The minimum deployment target is specified + as part of the ``-target *arch*-apple-macos*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=macos*version*`` + command-line argument. ``macosx`` is supported for backward-compatibility reasons, but it is deprecated. ``tvos`` - Apple's tvOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-tvos*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=tvos*version*`` + Apple's tvOS operating system. The minimum deployment target is specified + as part of the ``-target *arch*-apple-tvos*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=tvos*version*`` command-line argument. ``watchos`` Apple's watchOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-watchos*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=watchos*version*`` + as part of the ``-target *arch*-apple-watchos*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=watchos*version*`` command-line argument. ``visionos`` Apple's visionOS operating system. The minimum deployment target is specified - as part of the ``-target *arch*-apple-visionos*version*`` command line argument. - Alternatively, it can be specified by the ``-mtargetos=visionos*version*`` + as part of the ``-target *arch*-apple-visionos*version*`` command line argument. + Alternatively, it can be specified by the ``-mtargetos=visionos*version*`` command-line argument. ``driverkit`` Apple's DriverKit userspace kernel extensions. The minimum deployment target - is specified as part of the ``-target *arch*-apple-driverkit*version*`` + is specified as part of the ``-target *arch*-apple-driverkit*version*`` command line argument. A declaration can typically be used even when deploying back to a platform @@ -7522,7 +7522,7 @@ means that it can e.g no longer be part of an initializer expression. /* This may print something else than "6 * 7 = 42", if there is a non-weak definition of "ANSWER" in -an object linked in */ + an object linked in */ printf("6 * 7 = %d\n", ANSWER); return 0; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Harden security.cert.env.InvalidPtr checker fn matching (PR #88536)
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/88536 >From 915ab37028067fb38ffa69ae5c9726bb8c971436 Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Fri, 12 Apr 2024 19:07:49 +0200 Subject: [PATCH 1/2] [analyzer] Harden security.cert.env.InvalidPtr checker fn matching Fixes #88181 I'm also hardening an llvm::cast along the way. --- .../Checkers/cert/InvalidPtrChecker.cpp | 31 --- clang/test/Analysis/invalid-ptr-checker.cpp | 10 ++ 2 files changed, 30 insertions(+), 11 deletions(-) create mode 100644 clang/test/Analysis/invalid-ptr-checker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp index e5dd907c660d8e..fefe846b6911f7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp @@ -48,14 +48,19 @@ class InvalidPtrChecker bool InvalidatingGetEnv = false; // GetEnv can be treated invalidating and non-invalidating as well. - const CallDescription GetEnvCall{{"getenv"}, 1}; + const CallDescription GetEnvCall{CDM::CLibrary, {"getenv"}, 1}; const CallDescriptionMap EnvpInvalidatingFunctions = { - {{{"setenv"}, 3}, ::EnvpInvalidatingCall}, - {{{"unsetenv"}, 1}, ::EnvpInvalidatingCall}, - {{{"putenv"}, 1}, ::EnvpInvalidatingCall}, - {{{"_putenv_s"}, 2}, ::EnvpInvalidatingCall}, - {{{"_wputenv_s"}, 2}, ::EnvpInvalidatingCall}, + {{CDM::CLibrary, {"setenv"}, 3}, + ::EnvpInvalidatingCall}, + {{CDM::CLibrary, {"unsetenv"}, 1}, + ::EnvpInvalidatingCall}, + {{CDM::CLibrary, {"putenv"}, 1}, + ::EnvpInvalidatingCall}, + {{CDM::CLibrary, {"_putenv_s"}, 2}, + ::EnvpInvalidatingCall}, + {{CDM::CLibrary, {"_wputenv_s"}, 2}, + ::EnvpInvalidatingCall}, }; void postPreviousReturnInvalidatingCall(const CallEvent , @@ -63,13 +68,13 @@ class InvalidPtrChecker // SEI CERT ENV34-C const CallDescriptionMap PreviousCallInvalidatingFunctions = { - {{{"setlocale"}, 2}, + {{CDM::CLibrary, {"setlocale"}, 2}, ::postPreviousReturnInvalidatingCall}, - {{{"strerror"}, 1}, + {{CDM::CLibrary, {"strerror"}, 1}, ::postPreviousReturnInvalidatingCall}, - {{{"localeconv"}, 0}, + {{CDM::CLibrary, {"localeconv"}, 0}, ::postPreviousReturnInvalidatingCall}, - {{{"asctime"}, 1}, + {{CDM::CLibrary, {"asctime"}, 1}, ::postPreviousReturnInvalidatingCall}, }; @@ -205,8 +210,12 @@ void InvalidPtrChecker::postPreviousReturnInvalidatingCall( CE, LCtx, CE->getType(), C.blockCount()); State = State->BindExpr(CE, LCtx, RetVal); + const auto *SymRegOfRetVal = + dyn_cast_or_null(RetVal.getAsRegion()); + if (!SymRegOfRetVal) +return; + // Remember to this region. - const auto *SymRegOfRetVal = cast(RetVal.getAsRegion()); const MemRegion *MR = SymRegOfRetVal->getBaseRegion(); State = State->set(FD, MR); diff --git a/clang/test/Analysis/invalid-ptr-checker.cpp b/clang/test/Analysis/invalid-ptr-checker.cpp new file mode 100644 index 00..58bb45e0fb8421 --- /dev/null +++ b/clang/test/Analysis/invalid-ptr-checker.cpp @@ -0,0 +1,10 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,security.cert.env.InvalidPtr -verify %s + +// expected-no-diagnostics + +namespace other { +int strerror(int errnum); // custom strerror +void no_crash_on_custom_strerror() { + (void)strerror(0); // no-crash +} +} // namespace other >From 06d0056efd9616f76680ec7d923ed2bc76f2ab25 Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Sat, 13 Apr 2024 20:44:47 +0200 Subject: [PATCH 2/2] [docs] Add release notes for the crash fix --- clang/docs/ReleaseNotes.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 232de0d7d8bb73..7cb550cef62e64 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -600,6 +600,8 @@ Static Analyzer but not under any case blocks if ``unroll-loops=true`` analyzer config is set. (#GH68819) - Support C++23 static operator calls. (#GH84972) +- Fixed a crash in ``security.cert.env.InvalidPtr`` checker when accidentally + matched user-defined ``strerror`` and similar library functions. (GH#88181) New features ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Support wasm execution (PR #86402)
https://github.com/vgvassilev updated https://github.com/llvm/llvm-project/pull/86402 >From 4434ceeef152b95998ebd0a3b09a56d105490c4d Mon Sep 17 00:00:00 2001 From: Anubhab Ghosh Date: Sat, 23 Mar 2024 15:13:57 + Subject: [PATCH 1/2] [clang-repl] Support wasm execution. This commit introduces support for running clang-repl and executing C++ code interactively inside a Javascript engine using WebAssembly when built with Emscripten. This is achieved by producing WASM "shared libraries" that can be loaded by the Emscripten runtime using dlopen() More discussion is available in https://reviews.llvm.org/D158140 --- clang/lib/Interpreter/CMakeLists.txt | 1 + clang/lib/Interpreter/IncrementalExecutor.cpp | 2 + clang/lib/Interpreter/IncrementalExecutor.h | 11 +- clang/lib/Interpreter/Interpreter.cpp | 11 ++ clang/lib/Interpreter/WASM.cpp| 107 ++ clang/lib/Interpreter/WASM.h | 33 ++ 6 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 clang/lib/Interpreter/WASM.cpp create mode 100644 clang/lib/Interpreter/WASM.h diff --git a/clang/lib/Interpreter/CMakeLists.txt b/clang/lib/Interpreter/CMakeLists.txt index 9065f998f73c47..a8a287edf5b049 100644 --- a/clang/lib/Interpreter/CMakeLists.txt +++ b/clang/lib/Interpreter/CMakeLists.txt @@ -20,6 +20,7 @@ add_clang_library(clangInterpreter Interpreter.cpp InterpreterUtils.cpp Value.cpp + WASM.cpp DEPENDS intrinsics_gen diff --git a/clang/lib/Interpreter/IncrementalExecutor.cpp b/clang/lib/Interpreter/IncrementalExecutor.cpp index 6f036107c14a9c..1824a5b4570a93 100644 --- a/clang/lib/Interpreter/IncrementalExecutor.cpp +++ b/clang/lib/Interpreter/IncrementalExecutor.cpp @@ -36,6 +36,8 @@ LLVM_ATTRIBUTE_USED void linkComponents() { } namespace clang { +IncrementalExecutor::IncrementalExecutor(llvm::orc::ThreadSafeContext ) +: TSCtx(TSC) {} llvm::Expected> IncrementalExecutor::createDefaultJITBuilder( diff --git a/clang/lib/Interpreter/IncrementalExecutor.h b/clang/lib/Interpreter/IncrementalExecutor.h index b4347209e14fe3..7954cde36588bd 100644 --- a/clang/lib/Interpreter/IncrementalExecutor.h +++ b/clang/lib/Interpreter/IncrementalExecutor.h @@ -43,16 +43,19 @@ class IncrementalExecutor { llvm::DenseMap ResourceTrackers; +protected: + IncrementalExecutor(llvm::orc::ThreadSafeContext ); + public: enum SymbolNameKind { IRName, LinkerName }; IncrementalExecutor(llvm::orc::ThreadSafeContext , llvm::orc::LLJITBuilder , llvm::Error ); - ~IncrementalExecutor(); + virtual ~IncrementalExecutor(); - llvm::Error addModule(PartialTranslationUnit ); - llvm::Error removeModule(PartialTranslationUnit ); - llvm::Error runCtors() const; + virtual llvm::Error addModule(PartialTranslationUnit ); + virtual llvm::Error removeModule(PartialTranslationUnit ); + virtual llvm::Error runCtors() const; llvm::Error cleanUp(); llvm::Expected getSymbolAddress(llvm::StringRef Name, SymbolNameKind NameKind) const; diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp index cf31456b6950ac..7d572b20cd8281 100644 --- a/clang/lib/Interpreter/Interpreter.cpp +++ b/clang/lib/Interpreter/Interpreter.cpp @@ -15,6 +15,7 @@ #include "IncrementalExecutor.h" #include "IncrementalParser.h" #include "InterpreterUtils.h" +#include "WASM.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Mangle.h" @@ -183,6 +184,12 @@ IncrementalCompilerBuilder::CreateCpp() { std::vector Argv; Argv.reserve(5 + 1 + UserArgs.size()); Argv.push_back("-xc++"); +#ifdef __EMSCRIPTEN__ + Argv.push_back("-target"); + Argv.push_back("wasm32-unknown-emscripten"); + Argv.push_back("-pie"); + Argv.push_back("-shared"); +#endif Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end()); std::string TT = TargetTriple ? *TargetTriple : llvm::sys::getProcessTriple(); @@ -400,7 +407,11 @@ llvm::Error Interpreter::CreateExecutor() { if (!JB) return JB.takeError(); llvm::Error Err = llvm::Error::success(); +#ifdef __EMSCRIPTEN__ + auto Executor = std::make_unique(*TSCtx, **JB, Err); +#else auto Executor = std::make_unique(*TSCtx, **JB, Err); +#endif if (!Err) IncrExecutor = std::move(Executor); diff --git a/clang/lib/Interpreter/WASM.cpp b/clang/lib/Interpreter/WASM.cpp new file mode 100644 index 00..d21d0ada1eafac --- /dev/null +++ b/clang/lib/Interpreter/WASM.cpp @@ -0,0 +1,107 @@ +//===- WASM.cpp - WASM Interpreter --*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// This file implements interpreter support for code execution in WebAssembly. +//
[clang] [clang-scan-deps] Fix contention when updating `TrackingStatistic`s in hot code paths in `FileManager`. (PR #88427)
aganea wrote: I think in the short term @jansvoboda11's suggestion should be good enough. Bit if we want `Statistics` to be always cheap, we should make them `thread_local` instead, not atomic. `getValue()` could do the "collection" of data over all active, or past threads. It would also need a mechanism for collecting data when a thread ends through `pthread_key_create/FlsCallback`s. It would be a bit more involved than what's there currently, but that should fix the issue I'm seeing (and maybe others). https://github.com/llvm/llvm-project/pull/88427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Reduce the size of Decl and classes derived from it (PR #87361)
https://github.com/philnik777 updated https://github.com/llvm/llvm-project/pull/87361 >From b8a626116b0719c1acf75e9e7300df8e2bf82f99 Mon Sep 17 00:00:00 2001 From: Nikolas Klauser Date: Tue, 2 Apr 2024 18:00:05 +0200 Subject: [PATCH 1/3] [Clang] Reduce the size of Decl and classes derived from it --- clang/include/clang/AST/DeclBase.h| 66 ++- clang/lib/AST/DeclBase.cpp| 29 ++ clang/lib/Serialization/ASTReaderDecl.cpp | 2 +- 3 files changed, 62 insertions(+), 35 deletions(-) diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 47ed6d0d1db0df..172bd581b527c8 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -268,17 +268,34 @@ class alignas(8) Decl { /// } /// void A::f(); // SemanticDC == namespace 'A' ///// LexicalDC == global namespace - llvm::PointerUnion DeclCtx; + llvm::PointerIntPair< + llvm::PointerIntPair, 1, + bool>, + 1, bool> + DeclCtxWithInvalidDeclAndHasAttrs; - bool isInSemaDC() const { return DeclCtx.is(); } - bool isOutOfSemaDC() const { return DeclCtx.is(); } + bool isInSemaDC() const { +return DeclCtxWithInvalidDeclAndHasAttrs.getPointer() +.getPointer() +.is(); + } + + bool isOutOfSemaDC() const { +return DeclCtxWithInvalidDeclAndHasAttrs.getPointer() +.getPointer() +.is(); + } MultipleDC *getMultipleDC() const { -return DeclCtx.get(); +return DeclCtxWithInvalidDeclAndHasAttrs.getPointer() +.getPointer() +.get(); } DeclContext *getSemanticDC() const { -return DeclCtx.get(); +return DeclCtxWithInvalidDeclAndHasAttrs.getPointer() +.getPointer() +.get(); } /// Loc - The location of this decl. @@ -288,14 +305,6 @@ class alignas(8) Decl { LLVM_PREFERRED_TYPE(Kind) unsigned DeclKind : 7; - /// InvalidDecl - This indicates a semantic error occurred. - LLVM_PREFERRED_TYPE(bool) - unsigned InvalidDecl : 1; - - /// HasAttrs - This indicates whether the decl has attributes or not. - LLVM_PREFERRED_TYPE(bool) - unsigned HasAttrs : 1; - /// Implicit - Whether this declaration was implicitly generated by /// the implementation rather than explicitly written by the user. LLVM_PREFERRED_TYPE(bool) @@ -393,21 +402,22 @@ class alignas(8) Decl { protected: Decl(Kind DK, DeclContext *DC, SourceLocation L) : NextInContextAndBits(nullptr, getModuleOwnershipKindForChildOf(DC)), -DeclCtx(DC), Loc(L), DeclKind(DK), InvalidDecl(false), HasAttrs(false), -Implicit(false), Used(false), Referenced(false), +DeclCtxWithInvalidDeclAndHasAttrs({DC, false}, false), Loc(L), +DeclKind(DK), Implicit(false), Used(false), Referenced(false), TopLevelDeclInObjCContainer(false), Access(AS_none), FromASTFile(0), IdentifierNamespace(getIdentifierNamespaceForKind(DK)), CacheValidAndLinkage(llvm::to_underlying(Linkage::Invalid)) { -if (StatisticsEnabled) add(DK); +if (StatisticsEnabled) + add(DK); } Decl(Kind DK, EmptyShell Empty) - : DeclKind(DK), InvalidDecl(false), HasAttrs(false), Implicit(false), -Used(false), Referenced(false), TopLevelDeclInObjCContainer(false), -Access(AS_none), FromASTFile(0), + : DeclKind(DK), Implicit(false), Used(false), Referenced(false), +TopLevelDeclInObjCContainer(false), Access(AS_none), FromASTFile(0), IdentifierNamespace(getIdentifierNamespaceForKind(DK)), CacheValidAndLinkage(llvm::to_underlying(Linkage::Invalid)) { -if (StatisticsEnabled) add(DK); +if (StatisticsEnabled) + add(DK); } virtual ~Decl(); @@ -520,7 +530,7 @@ class alignas(8) Decl { return AccessSpecifier(Access); } - bool hasAttrs() const { return HasAttrs; } + bool hasAttrs() const { return DeclCtxWithInvalidDeclAndHasAttrs.getPointer().getInt(); } void setAttrs(const AttrVec& Attrs) { return setAttrsImpl(Attrs, getASTContext()); @@ -549,13 +559,16 @@ class alignas(8) Decl { } template void dropAttrs() { -if (!HasAttrs) return; +if (!hasAttrs()) return; AttrVec = getAttrs(); llvm::erase_if(Vec, [](Attr *A) { return isa(A); }); -if (Vec.empty()) - HasAttrs = false; +if (Vec.empty()) { + auto InnerPtr = DeclCtxWithInvalidDeclAndHasAttrs.getPointer(); + InnerPtr.setInt(false); + DeclCtxWithInvalidDeclAndHasAttrs.setPointer(InnerPtr); +} } template void dropAttr() { dropAttrs(); } @@ -590,7 +603,10 @@ class alignas(8) Decl { /// setInvalidDecl - Indicates the Decl had a semantic error. This /// allows for graceful error recovery. void setInvalidDecl(bool Invalid = true); - bool isInvalidDecl() const { return (bool) InvalidDecl; } + + bool isInvalidDecl() const { +return DeclCtxWithInvalidDeclAndHasAttrs.getInt(); + }
[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)
kimgr wrote: > I am wondering if that'd be interesting and if so, maybe we can share it > between both projects via upstreaming to Clang... That sounds fantastic, but mostly because I don't have anything to offer, and everything to benefit :) I was just thinking about adding a `.ForwardDeclaration` policy to `DeclPrinter` this morning -- the current `PolishForDeclaration` output is _so_ close. But it seemed weird to add a mode for which IWYU (a non-LLVM project) was the only user. I suspect your `ForwardDeclarePrinter` is more principled. I guess it might be hard to coordinate forward-decl style between users, but ultimately that's just printing policy (or chaining libFormat, or something). Let's maybe continue this discussion somewhere else? https://github.com/llvm/llvm-project/pull/88600 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang Format] Fixing erroneous statements in tests; nsc (PR #88628)
https://github.com/edwardbottom edited https://github.com/llvm/llvm-project/pull/88628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update FormatTest.cpp (PR #88628)
llvmbot wrote: @llvm/pr-subscribers-clang-format Author: Edward Bottom (edwardbottom) Changes --- Full diff: https://github.com/llvm/llvm-project/pull/88628.diff 1 Files Affected: - (modified) clang/unittests/Format/FormatTest.cpp (+4-4) ``diff diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 4906b3350b5b22..91dfa5a4b61d7f 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -5434,7 +5434,7 @@ TEST_F(FormatTest, IndentsPPDirectiveWithPPIndentWidth) { "C();\n" "#endif", style); - verifyFormat("if (emacs) {\n" + verifyFormat("if (vim) {\n" "#ifdef is\n" "#define lit \\\n" "if (af) { \\\n" @@ -5457,7 +5457,7 @@ TEST_F(FormatTest, IndentsPPDirectiveWithPPIndentWidth) { style); verifyFormat("#ifndef foo\n" "#define foo\n" - "if (emacs) {\n" + "if (vim) {\n" "#ifdef is\n" "#define lit \\\n" "if (af) { \\\n" @@ -5543,7 +5543,7 @@ TEST_F(FormatTest, IndentsPPDirectiveWithPPIndentWidth) { style); verifyFormat("#ifndef foo\n" "#define foo\n" - "if (emacs) {\n" + "if (vim) {\n" "#ifdef is\n" "# define lit \\\n" " if (af) { \\\n" @@ -5583,7 +5583,7 @@ TEST_F(FormatTest, IndentsPPDirectiveWithPPIndentWidth) { style.IndentWidth = 4; style.PPIndentWidth = 1; style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash; - verifyFormat("if (emacs) {\n" + verifyFormat("if (vim) {\n" "#ifdef is\n" " #define lit \\\n" " if (af) { \\\n" `` https://github.com/llvm/llvm-project/pull/88628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update FormatTest.cpp (PR #88628)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/88628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Update FormatTest.cpp (PR #88628)
https://github.com/edwardbottom created https://github.com/llvm/llvm-project/pull/88628 None >From 22ae52878602dfc79ab90bbc6e9dea508c3762e2 Mon Sep 17 00:00:00 2001 From: Edward Bottom <31777866+edwardbot...@users.noreply.github.com> Date: Sat, 13 Apr 2024 08:49:47 -0700 Subject: [PATCH] Update FormatTest.cpp --- clang/unittests/Format/FormatTest.cpp | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 4906b3350b5b22..91dfa5a4b61d7f 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -5434,7 +5434,7 @@ TEST_F(FormatTest, IndentsPPDirectiveWithPPIndentWidth) { "C();\n" "#endif", style); - verifyFormat("if (emacs) {\n" + verifyFormat("if (vim) {\n" "#ifdef is\n" "#define lit \\\n" "if (af) { \\\n" @@ -5457,7 +5457,7 @@ TEST_F(FormatTest, IndentsPPDirectiveWithPPIndentWidth) { style); verifyFormat("#ifndef foo\n" "#define foo\n" - "if (emacs) {\n" + "if (vim) {\n" "#ifdef is\n" "#define lit \\\n" "if (af) { \\\n" @@ -5543,7 +5543,7 @@ TEST_F(FormatTest, IndentsPPDirectiveWithPPIndentWidth) { style); verifyFormat("#ifndef foo\n" "#define foo\n" - "if (emacs) {\n" + "if (vim) {\n" "#ifdef is\n" "# define lit \\\n" " if (af) { \\\n" @@ -5583,7 +5583,7 @@ TEST_F(FormatTest, IndentsPPDirectiveWithPPIndentWidth) { style.IndentWidth = 4; style.PPIndentWidth = 1; style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash; - verifyFormat("if (emacs) {\n" + verifyFormat("if (vim) {\n" "#ifdef is\n" " #define lit \\\n" " if (af) { \\\n" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)
kimgr wrote: Current PR passes all my tests, both Clang (`ninja ASTTests`), Clangd (`ninja ClangdTests`) and IWYU end-to-end tests -- thanks! https://github.com/llvm/llvm-project/pull/88600 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)
vgvassilev wrote: > * IWYU uses `PolishForDeclaration` to get a valid _declaration_, and then > does [some very hacky heuristic > post-processing](https://github.com/include-what-you-use/include-what-you-use/blob/125341c412ceee9233ece8973848b49e770a9b82/iwyu_output.cc#L469) > to turn it into a forward-decl. The [ROOT](http://root.cern) usecase of the Cling interpreter has similar infrastructure and it works at scale couple of million lines of scientific codes: https://github.com/root-project/root/blob/master/interpreter/cling/lib/Interpreter/ForwardDeclPrinter.cpp. There are some tests that one can look at: https://github.com/root-project/root/tree/master/interpreter/cling/test/Autoloading I am wondering if that'd be interesting and if so, maybe we can share it between both projects via upstreaming to Clang... https://github.com/llvm/llvm-project/pull/88600 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)
@@ -252,16 +252,19 @@ static DeclPrinter::AttrPosAsWritten getPosAsWritten(const Attr *A, return DeclPrinter::AttrPosAsWritten::Right; } -void DeclPrinter::prettyPrintAttributes(const Decl *D, +// returns true if an attribute was printed. +bool DeclPrinter::prettyPrintAttributes(const Decl *D, AttrPosAsWritten Pos /*=Default*/) { - if (Policy.PolishForDeclaration) -return; + bool hasPrinted = false; if (D->hasAttrs()) { const AttrVec = D->getAttrs(); for (auto *A : Attrs) { if (A->isInherited() || A->isImplicit()) continue; + // Don't strip out the keyword attributes, they aren't regular attributes. + if (Policy.PolishForDeclaration && !A->isKeywordAttribute()) kimgr wrote: Nice, I was about to suggest something like this, but didn't know `isKeywordAttribute` existed. https://github.com/llvm/llvm-project/pull/88600 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)
kimgr wrote: > If the intent is to produce a forward declaration the final keyword cannot be > attached to a forward declaration. So I am not sure what's the "right" fix > here... I don't believe that's the intent of `DeclPrinter` or `PolishForDeclaration` -- * Clangd uses `PolishForDeclaration` to print an informational "hover text" for the declaration (and I guess that's why they want to include `final` -- it's something that's good for an interactive user to know about the decl) * IWYU uses `PolishForDeclaration` to get a valid _declaration_, and then does [some very hacky heuristic post-processing](https://github.com/include-what-you-use/include-what-you-use/blob/125341c412ceee9233ece8973848b49e770a9b82/iwyu_output.cc#L469) to turn it into a forward-decl. Sorry for stirring confusion into this, my primary focus is IWYU, but I remember that the original double-final came from a change intended for Clangd. Hopefully this helps clarify. https://github.com/llvm/llvm-project/pull/88600 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)
https://github.com/vgvassilev updated https://github.com/llvm/llvm-project/pull/88600 >From 9b2bb9068cbefcfffd0931fbbee46b1a0f536a4f Mon Sep 17 00:00:00 2001 From: Vassil Vassilev Date: Sat, 13 Apr 2024 06:39:34 + Subject: [PATCH] Fix the double space and double attribute printing of the final keyword. Fixes #56517. --- clang/lib/AST/DeclPrinter.cpp | 36 +++- clang/test/SemaCXX/cxx11-attr-print.cpp | 5 +++ clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl | 4 +-- clang/unittests/AST/DeclPrinterTest.cpp | 37 - clang/utils/TableGen/ClangAttrEmitter.cpp | 2 +- 5 files changed, 57 insertions(+), 27 deletions(-) diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp index 6afdb6cfccb142..444780947f2075 100644 --- a/clang/lib/AST/DeclPrinter.cpp +++ b/clang/lib/AST/DeclPrinter.cpp @@ -119,7 +119,7 @@ namespace { void printTemplateArguments(llvm::ArrayRef Args, const TemplateParameterList *Params); enum class AttrPosAsWritten { Default = 0, Left, Right }; -void +bool prettyPrintAttributes(const Decl *D, AttrPosAsWritten Pos = AttrPosAsWritten::Default); void prettyPrintPragmas(Decl *D); @@ -252,16 +252,19 @@ static DeclPrinter::AttrPosAsWritten getPosAsWritten(const Attr *A, return DeclPrinter::AttrPosAsWritten::Right; } -void DeclPrinter::prettyPrintAttributes(const Decl *D, +// returns true if an attribute was printed. +bool DeclPrinter::prettyPrintAttributes(const Decl *D, AttrPosAsWritten Pos /*=Default*/) { - if (Policy.PolishForDeclaration) -return; + bool hasPrinted = false; if (D->hasAttrs()) { const AttrVec = D->getAttrs(); for (auto *A : Attrs) { if (A->isInherited() || A->isImplicit()) continue; + // Don't strip out the keyword attributes, they aren't regular attributes. + if (Policy.PolishForDeclaration && !A->isKeywordAttribute()) +continue; switch (A->getKind()) { #define ATTR(X) #define PRAGMA_SPELLING_ATTR(X) case attr::X: @@ -275,6 +278,7 @@ void DeclPrinter::prettyPrintAttributes(const Decl *D, if (Pos != AttrPosAsWritten::Left) Out << ' '; A->printPretty(Out, Policy); + hasPrinted = true; if (Pos == AttrPosAsWritten::Left) Out << ' '; } @@ -282,6 +286,7 @@ void DeclPrinter::prettyPrintAttributes(const Decl *D, } } } + return hasPrinted; } void DeclPrinter::prettyPrintPragmas(Decl *D) { @@ -1060,12 +1065,15 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) { // FIXME: add printing of pragma attributes if required. if (!Policy.SuppressSpecifiers && D->isModulePrivate()) Out << "__module_private__ "; - Out << D->getKindName(); - prettyPrintAttributes(D); + Out << D->getKindName() << ' '; - if (D->getIdentifier()) { + // FIXME: Move before printing the decl kind to match the behavior of the + // attribute printing for variables and function where they are printed first. + if (prettyPrintAttributes(D, AttrPosAsWritten::Left)) Out << ' '; + + if (D->getIdentifier()) { if (auto *NNS = D->getQualifier()) NNS->print(Out, Policy); Out << *D; @@ -1082,16 +1090,13 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) { } } - if (D->hasDefinition()) { -if (D->hasAttr()) { - Out << " final"; -} - } + prettyPrintAttributes(D, AttrPosAsWritten::Right); if (D->isCompleteDefinition()) { +Out << ' '; // Print the base classes if (D->getNumBases()) { - Out << " : "; + Out << ": "; for (CXXRecordDecl::base_class_iterator Base = D->bases_begin(), BaseEnd = D->bases_end(); Base != BaseEnd; ++Base) { if (Base != D->bases_begin()) @@ -1110,14 +1115,15 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) { if (Base->isPackExpansion()) Out << "..."; } + Out << ' '; } // Print the class definition // FIXME: Doesn't print access specifiers, e.g., "public:" if (Policy.TerseOutput) { - Out << " {}"; + Out << "{}"; } else { - Out << " {\n"; + Out << "{\n"; VisitDeclContext(D); Indent() << "}"; } diff --git a/clang/test/SemaCXX/cxx11-attr-print.cpp b/clang/test/SemaCXX/cxx11-attr-print.cpp index a169d1b4409b4d..2b084018bc0662 100644 --- a/clang/test/SemaCXX/cxx11-attr-print.cpp +++ b/clang/test/SemaCXX/cxx11-attr-print.cpp @@ -87,3 +87,8 @@ template struct S; // CHECK: using Small2 {{\[}}[gnu::mode(byte)]] = int; using Small2 [[gnu::mode(byte)]] = int; + +class FinalNonTemplate final {}; +// CHECK: class FinalNonTemplate final { +template class FinalTemplate final {}; +// CHECK: template class FinalTemplate final { diff --git a/clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl
[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)
https://github.com/vgvassilev updated https://github.com/llvm/llvm-project/pull/88600 >From cb3da95dd80c5ed991c5342e655e0f170eab16eb Mon Sep 17 00:00:00 2001 From: Vassil Vassilev Date: Sat, 13 Apr 2024 06:39:34 + Subject: [PATCH] Fix the double space and double attribute printing of the final keyword. Fixes #56517. --- clang/lib/AST/DeclPrinter.cpp | 36 +++- clang/test/SemaCXX/cxx11-attr-print.cpp | 5 +++ clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl | 4 +-- clang/unittests/AST/DeclPrinterTest.cpp | 37 - clang/utils/TableGen/ClangAttrEmitter.cpp | 2 +- 5 files changed, 57 insertions(+), 27 deletions(-) diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp index 6afdb6cfccb142..f5ae5d5b36449a 100644 --- a/clang/lib/AST/DeclPrinter.cpp +++ b/clang/lib/AST/DeclPrinter.cpp @@ -119,7 +119,7 @@ namespace { void printTemplateArguments(llvm::ArrayRef Args, const TemplateParameterList *Params); enum class AttrPosAsWritten { Default = 0, Left, Right }; -void +bool prettyPrintAttributes(const Decl *D, AttrPosAsWritten Pos = AttrPosAsWritten::Default); void prettyPrintPragmas(Decl *D); @@ -252,16 +252,19 @@ static DeclPrinter::AttrPosAsWritten getPosAsWritten(const Attr *A, return DeclPrinter::AttrPosAsWritten::Right; } -void DeclPrinter::prettyPrintAttributes(const Decl *D, +// returns true if an attribute was printed. +bool DeclPrinter::prettyPrintAttributes(const Decl *D, AttrPosAsWritten Pos /*=Default*/) { - if (Policy.PolishForDeclaration) -return; + bool hasPrinted = false; if (D->hasAttrs()) { const AttrVec = D->getAttrs(); for (auto *A : Attrs) { if (A->isInherited() || A->isImplicit()) continue; + // Don't strip out the keyword attributes, they aren't regular attributes. + if (Policy.PolishForDeclaration && A->isKeywordAttribute()) +continue; switch (A->getKind()) { #define ATTR(X) #define PRAGMA_SPELLING_ATTR(X) case attr::X: @@ -275,6 +278,7 @@ void DeclPrinter::prettyPrintAttributes(const Decl *D, if (Pos != AttrPosAsWritten::Left) Out << ' '; A->printPretty(Out, Policy); + hasPrinted = true; if (Pos == AttrPosAsWritten::Left) Out << ' '; } @@ -282,6 +286,7 @@ void DeclPrinter::prettyPrintAttributes(const Decl *D, } } } + return hasPrinted; } void DeclPrinter::prettyPrintPragmas(Decl *D) { @@ -1060,12 +1065,15 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) { // FIXME: add printing of pragma attributes if required. if (!Policy.SuppressSpecifiers && D->isModulePrivate()) Out << "__module_private__ "; - Out << D->getKindName(); - prettyPrintAttributes(D); + Out << D->getKindName() << ' '; - if (D->getIdentifier()) { + // FIXME: Move before printing the decl kind to match the behavior of the + // attribute printing for variables and function where they are printed first. + if (prettyPrintAttributes(D, AttrPosAsWritten::Left)) Out << ' '; + + if (D->getIdentifier()) { if (auto *NNS = D->getQualifier()) NNS->print(Out, Policy); Out << *D; @@ -1082,16 +1090,13 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) { } } - if (D->hasDefinition()) { -if (D->hasAttr()) { - Out << " final"; -} - } + prettyPrintAttributes(D, AttrPosAsWritten::Right); if (D->isCompleteDefinition()) { +Out << ' '; // Print the base classes if (D->getNumBases()) { - Out << " : "; + Out << ": "; for (CXXRecordDecl::base_class_iterator Base = D->bases_begin(), BaseEnd = D->bases_end(); Base != BaseEnd; ++Base) { if (Base != D->bases_begin()) @@ -1110,14 +1115,15 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) { if (Base->isPackExpansion()) Out << "..."; } + Out << ' '; } // Print the class definition // FIXME: Doesn't print access specifiers, e.g., "public:" if (Policy.TerseOutput) { - Out << " {}"; + Out << "{}"; } else { - Out << " {\n"; + Out << "{\n"; VisitDeclContext(D); Indent() << "}"; } diff --git a/clang/test/SemaCXX/cxx11-attr-print.cpp b/clang/test/SemaCXX/cxx11-attr-print.cpp index a169d1b4409b4d..2b084018bc0662 100644 --- a/clang/test/SemaCXX/cxx11-attr-print.cpp +++ b/clang/test/SemaCXX/cxx11-attr-print.cpp @@ -87,3 +87,8 @@ template struct S; // CHECK: using Small2 {{\[}}[gnu::mode(byte)]] = int; using Small2 [[gnu::mode(byte)]] = int; + +class FinalNonTemplate final {}; +// CHECK: class FinalNonTemplate final { +template class FinalTemplate final {}; +// CHECK: template class FinalTemplate final { diff --git a/clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl
[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)
vgvassilev wrote: I've put a fix that fixes the cases that you mentioned... https://github.com/llvm/llvm-project/pull/88600 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)
https://github.com/vgvassilev updated https://github.com/llvm/llvm-project/pull/88600 >From c68344d2d2f22f88ef386f655cc7698759fb551c Mon Sep 17 00:00:00 2001 From: Vassil Vassilev Date: Sat, 13 Apr 2024 06:39:34 + Subject: [PATCH] Fix the double space and double attribute printing of the final keyword. Fixes #56517. --- clang/lib/AST/DeclPrinter.cpp | 36 +++- clang/test/SemaCXX/cxx11-attr-print.cpp | 5 +++ clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl | 4 +-- clang/unittests/AST/DeclPrinterTest.cpp | 37 - clang/utils/TableGen/ClangAttrEmitter.cpp | 2 +- 5 files changed, 57 insertions(+), 27 deletions(-) diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp index 6afdb6cfccb142..5d15541a6d54c5 100644 --- a/clang/lib/AST/DeclPrinter.cpp +++ b/clang/lib/AST/DeclPrinter.cpp @@ -119,7 +119,7 @@ namespace { void printTemplateArguments(llvm::ArrayRef Args, const TemplateParameterList *Params); enum class AttrPosAsWritten { Default = 0, Left, Right }; -void +bool prettyPrintAttributes(const Decl *D, AttrPosAsWritten Pos = AttrPosAsWritten::Default); void prettyPrintPragmas(Decl *D); @@ -252,16 +252,19 @@ static DeclPrinter::AttrPosAsWritten getPosAsWritten(const Attr *A, return DeclPrinter::AttrPosAsWritten::Right; } -void DeclPrinter::prettyPrintAttributes(const Decl *D, +// returns true if an attribute was printed. +bool DeclPrinter::prettyPrintAttributes(const Decl *D, AttrPosAsWritten Pos /*=Default*/) { - if (Policy.PolishForDeclaration) -return; + bool hasPrinted = false; if (D->hasAttrs()) { const AttrVec = D->getAttrs(); for (auto *A : Attrs) { if (A->isInherited() || A->isImplicit()) continue; + if (Policy.PolishForDeclaration && + A->getSyntax() != AttributeCommonInfo::AS_Keyword) +continue; switch (A->getKind()) { #define ATTR(X) #define PRAGMA_SPELLING_ATTR(X) case attr::X: @@ -275,6 +278,7 @@ void DeclPrinter::prettyPrintAttributes(const Decl *D, if (Pos != AttrPosAsWritten::Left) Out << ' '; A->printPretty(Out, Policy); + hasPrinted = true; if (Pos == AttrPosAsWritten::Left) Out << ' '; } @@ -282,6 +286,7 @@ void DeclPrinter::prettyPrintAttributes(const Decl *D, } } } + return hasPrinted; } void DeclPrinter::prettyPrintPragmas(Decl *D) { @@ -1060,12 +1065,15 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) { // FIXME: add printing of pragma attributes if required. if (!Policy.SuppressSpecifiers && D->isModulePrivate()) Out << "__module_private__ "; - Out << D->getKindName(); - prettyPrintAttributes(D); + Out << D->getKindName() << ' '; - if (D->getIdentifier()) { + // FIXME: Move before printing the decl kind to match the behavior of the + // attribute printing for variables and function where they are printed first. + if (prettyPrintAttributes(D, AttrPosAsWritten::Left)) Out << ' '; + + if (D->getIdentifier()) { if (auto *NNS = D->getQualifier()) NNS->print(Out, Policy); Out << *D; @@ -1082,16 +1090,13 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) { } } - if (D->hasDefinition()) { -if (D->hasAttr()) { - Out << " final"; -} - } + prettyPrintAttributes(D, AttrPosAsWritten::Right); if (D->isCompleteDefinition()) { +Out << ' '; // Print the base classes if (D->getNumBases()) { - Out << " : "; + Out << ": "; for (CXXRecordDecl::base_class_iterator Base = D->bases_begin(), BaseEnd = D->bases_end(); Base != BaseEnd; ++Base) { if (Base != D->bases_begin()) @@ -1110,14 +1115,15 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) { if (Base->isPackExpansion()) Out << "..."; } + Out << ' '; } // Print the class definition // FIXME: Doesn't print access specifiers, e.g., "public:" if (Policy.TerseOutput) { - Out << " {}"; + Out << "{}"; } else { - Out << " {\n"; + Out << "{\n"; VisitDeclContext(D); Indent() << "}"; } diff --git a/clang/test/SemaCXX/cxx11-attr-print.cpp b/clang/test/SemaCXX/cxx11-attr-print.cpp index a169d1b4409b4d..2b084018bc0662 100644 --- a/clang/test/SemaCXX/cxx11-attr-print.cpp +++ b/clang/test/SemaCXX/cxx11-attr-print.cpp @@ -87,3 +87,8 @@ template struct S; // CHECK: using Small2 {{\[}}[gnu::mode(byte)]] = int; using Small2 [[gnu::mode(byte)]] = int; + +class FinalNonTemplate final {}; +// CHECK: class FinalNonTemplate final { +template class FinalTemplate final {}; +// CHECK: template class FinalTemplate final { diff --git a/clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl b/clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl index