[PATCH] D151145: Add disabled unittest reproducing TextProto formatting issue.
Ben J via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu May 25 08:17:47 PDT 2023
Icantjuddle added a comment.
In D151145#4369406 <https://reviews.llvm.org/D151145#4369406>, @MyDeveloperDay wrote:
> We don't normally land broken tests, even if they are disabled, its better for us if we get a fix at the same time ;-)
In D151145#4367495 <https://reviews.llvm.org/D151145#4367495>, @krasimir wrote:
> ...
> @Icantjuddle would you like to update this patch to fix the issue and enable your newly added tests?
Sounds like we don't really want this without a real fix; I'll give it a shot, hopefully won't be too bad. Thanks for the code pointer.
In D151145#4365580 <https://reviews.llvm.org/D151145#4365580>, @HazardyKnusperkeks wrote:
> As stated on Github I don't think we have a bug.
Are you sure, I'm pretty sure this is a bug? Can you elaborate on the discussion on github. I wouldn't want to fix something that isn't broken. If this is indeed expected maybe I can update the docs.
================
Comment at: clang/unittests/Format/FormatTestRawStrings.cpp:92-99
Style.RawStringFormats = {
{
/*Language=*/FormatStyle::LK_Cpp,
/*Delimiters=*/{"cpp"},
/*EnclosingFunctions=*/{},
/*CanonicalDelimiter=*/"",
BasedOnStyle,
----------------
HazardyKnusperkeks wrote:
> Couldn't you just set the style like you want, instead of parsing yaml?
Given that the problem seems to be with how the configs are parsed/processed, I wanted the repro to be most faithful to the user facing issue.
================
Comment at: clang/unittests/Format/FormatTestRawStrings.cpp:177
+ format(R"test(
+t = R"pb(item:1)pb";)test",
+ getRawStringPbStyleSeparateSection()));
----------------
HazardyKnusperkeks wrote:
> EXPECT_EQ
I used the function instead of the macro to conform to the convention of every other test in the file.
The function provides its own justification.
```
// Gcc 4.8 doesn't support raw string literals in macros, which breaks some
// build bots. We use this function instead.
void expect_eq(const std::string Expected, const std::string Actual) {
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151145/new/
https://reviews.llvm.org/D151145
More information about the cfe-commits
mailing list