[clang] Fixes and closes #53952 (PR #68127)

2023-10-03 Thread Rajkumar Ananthu via cfe-commits 

https://github.com/rajkumarananthu created 
https://github.com/llvm/llvm-project/pull/68127

The issue #53952 is reported indicating clang is giving a crashing pch file, 
when hasErrors is been passed incorrectly to WriteAST method.

To fix the issue, I have a added an assertion to make sure the given value of 
ASTHasCompilerErrors is matching with Preprocessor diagnostics. And this 
assertion will get triggered inside Debug builds.

For release builds, based on the conditional check, forcefully set the 
ASTHasCompilerErrors member variable to a valid value from Preprocessor.

>From b76f4d351b3e55e2095e3be088a50fdb76d6b7f9 Mon Sep 17 00:00:00 2001
From: Rajkumar Ananthu 
Date: Tue, 3 Oct 2023 21:53:59 +0530
Subject: [PATCH] Fixes and closes #53952

The issue #53952 is reported indicating clang is giving a crashing pch file, 
when hasErrors is been passed incorrectly to WriteAST method.

To fix the issue, I have a added an assertion to make sure the given value of 
ASTHasCompilerErrors is matching with Preprocessor diagnostics. And this 
assertion will get triggered inside Debug builds.

For release builds, based on the conditional check, forcefully set the 
ASTHasCompilerErrors member variable to a valid value from Preprocessor.
---
 clang/lib/Serialization/ASTWriter.cpp | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 201e2fcaaec91aa..35f37de9b1850ad 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4628,6 +4628,12 @@ ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef, 
StringRef OutputFile,
   WritingAST = true;
 
   ASTHasCompilerErrors = hasErrors;
+  bool trueHasErrors = 
SemaRef.PP.getDiagnostics().hasUncompilableErrorOccurred();
+  assert(ASTHasCompilerErrors == trueHasErrors);
+  if (trueHasErrors != ASTHasCompilerErrors) {
+  // forcing the compiler errors flag to be set correctly
+  ASTHasCompilerErrors = trueHasErrors;
+  }
 
   // Emit the file header.
   Stream.Emit((unsigned)'C', 8);

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fixes and closes #53952. Setting the ASTHasCompilerErrors member variable correctly based on the PP diagnostics. (PR #68127)

2023-10-03 Thread Rajkumar Ananthu via cfe-commits 

https://github.com/rajkumarananthu edited 
https://github.com/llvm/llvm-project/pull/68127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fixes and closes #53952. Setting the ASTHasCompilerErrors member variable correctly based on the PP diagnostics. (PR #68127)

2023-10-03 Thread Rajkumar Ananthu via cfe-commits 


@@ -4628,6 +4628,12 @@ ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef, 
StringRef OutputFile,
   WritingAST = true;
 
   ASTHasCompilerErrors = hasErrors;
+  bool trueHasErrors = 
SemaRef.PP.getDiagnostics().hasUncompilableErrorOccurred();

rajkumarananthu wrote:

Hi @shafik, thanks for your time for the review.

The scenario here is that, at line 4630 you can see the `ASTHasCompilerErrors` 
is being set using `hasErrors` which is an input to the method here 
`ASTWriter::WriteAST()`.

And if you see the description of the issue reported, the user is purposely 
passing the `hasErrors` as `false` even when there is a compiler error in the 
input file, because of this clang is giving a crashing pch file.

So, if the `hasErrors` is not a valid value, I am setting it to the correct 
value. May be I can just remove all the unnecessary code and just directly 
assign `ASTHasCompilerErrors = 
SemaRef.PP.getDiagnostics().hasUncompilableErrorOccured();` which will be 
correct.

And assertion I have added to detect this in debug build early, may be I am 
wrong, sorry for the confusion, it should not behave differently in release and 
debug builds.

Let me know what you think of this?

Thanks
Rajkumar Ananthu.

https://github.com/llvm/llvm-project/pull/68127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fixes and closes #53952. Setting the ASTHasCompilerErrors member variable correctly based on the PP diagnostics. (PR #68127)

2023-10-03 Thread Rajkumar Ananthu via cfe-commits 

https://github.com/rajkumarananthu edited 
https://github.com/llvm/llvm-project/pull/68127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fixes and closes #53952. Setting the ASTHasCompilerErrors member variable correctly based on the PP diagnostics. (PR #68127)

2023-10-04 Thread Rajkumar Ananthu via cfe-commits 


@@ -4628,6 +4628,12 @@ ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef, 
StringRef OutputFile,
   WritingAST = true;
 
   ASTHasCompilerErrors = hasErrors;
+  bool trueHasErrors = 
SemaRef.PP.getDiagnostics().hasUncompilableErrorOccurred();

rajkumarananthu wrote:

Yeah @AaronBallman, that does sound better, I will remove the argument and 
update the pull request. Thanks for the review.

https://github.com/llvm/llvm-project/pull/68127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fixes and closes #53952. Setting the ASTHasCompilerErrors member variable correctly based on the PP diagnostics. (PR #68127)

2023-10-04 Thread Rajkumar Ananthu via cfe-commits 

https://github.com/rajkumarananthu updated 
https://github.com/llvm/llvm-project/pull/68127

>From b76f4d351b3e55e2095e3be088a50fdb76d6b7f9 Mon Sep 17 00:00:00 2001
From: Rajkumar Ananthu 
Date: Tue, 3 Oct 2023 21:53:59 +0530
Subject: [PATCH 1/2] Fixes and closes #53952

The issue #53952 is reported indicating clang is giving a crashing pch file, 
when hasErrors is been passed incorrectly to WriteAST method.

To fix the issue, I have a added an assertion to make sure the given value of 
ASTHasCompilerErrors is matching with Preprocessor diagnostics. And this 
assertion will get triggered inside Debug builds.

For release builds, based on the conditional check, forcefully set the 
ASTHasCompilerErrors member variable to a valid value from Preprocessor.
---
 clang/lib/Serialization/ASTWriter.cpp | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 201e2fcaaec91aa..35f37de9b1850ad 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4628,6 +4628,12 @@ ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef, 
StringRef OutputFile,
   WritingAST = true;
 
   ASTHasCompilerErrors = hasErrors;
+  bool trueHasErrors = 
SemaRef.PP.getDiagnostics().hasUncompilableErrorOccurred();
+  assert(ASTHasCompilerErrors == trueHasErrors);
+  if (trueHasErrors != ASTHasCompilerErrors) {
+  // forcing the compiler errors flag to be set correctly
+  ASTHasCompilerErrors = trueHasErrors;
+  }
 
   // Emit the file header.
   Stream.Emit((unsigned)'C', 8);

>From bd8a37df74b5bcac05d7eb7d74986d8c5095bb10 Mon Sep 17 00:00:00 2001
From: Rajkumar Ananthu 
Date: Thu, 5 Oct 2023 01:29:32 +0530
Subject: [PATCH 2/2] Removing hasErrors argument from ASTWriter::WriteAST
 method as suggested by the review

---
 clang/include/clang/Serialization/ASTWriter.h |  1 -
 clang/lib/Frontend/ASTUnit.cpp| 17 +
 clang/lib/Serialization/ASTWriter.cpp | 10 ++
 clang/lib/Serialization/GeneratePCH.cpp   |  8 ++--
 4 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index f2c7c03ff093607..98445d40ebd82c3 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -613,7 +613,6 @@ class ASTWriter : public ASTDeserializationListener,
   /// the module but currently is merely a random 32-bit number.
   ASTFileSignature WriteAST(Sema &SemaRef, StringRef OutputFile,
 Module *WritingModule, StringRef isysroot,
-bool hasErrors = false,
 bool ShouldCacheASTInMemory = false);
 
   /// Emit a token.
diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index 016f88a43a56ddd..85157c3b21b6648 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -2341,12 +2341,9 @@ bool ASTUnit::Save(StringRef File) {
   return false;
 }
 
-static bool serializeUnit(ASTWriter &Writer,
-  SmallVectorImpl &Buffer,
-  Sema &S,
-  bool hasErrors,
-  raw_ostream &OS) {
-  Writer.WriteAST(S, std::string(), nullptr, "", hasErrors);
+static bool serializeUnit(ASTWriter &Writer, SmallVectorImpl &Buffer,
+  Sema &S, raw_ostream &OS) {
+  Writer.WriteAST(S, std::string(), nullptr, "");
 
   // Write the generated bitstream to "Out".
   if (!Buffer.empty())
@@ -2356,18 +2353,14 @@ static bool serializeUnit(ASTWriter &Writer,
 }
 
 bool ASTUnit::serialize(raw_ostream &OS) {
-  // For serialization we are lenient if the errors were only warn-as-error 
kind.
-  bool hasErrors = getDiagnostics().hasUncompilableErrorOccurred();
-
   if (WriterData)
-return serializeUnit(WriterData->Writer, WriterData->Buffer,
- getSema(), hasErrors, OS);
+return serializeUnit(WriterData->Writer, WriterData->Buffer, getSema(), 
OS);
 
   SmallString<128> Buffer;
   llvm::BitstreamWriter Stream(Buffer);
   InMemoryModuleCache ModuleCache;
   ASTWriter Writer(Stream, Buffer, ModuleCache, {});
-  return serializeUnit(Writer, Buffer, getSema(), hasErrors, OS);
+  return serializeUnit(Writer, Buffer, getSema(), OS);
 }
 
 using SLocRemap = ContinuousRangeMap;
diff --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 35f37de9b1850ad..0acd86de06ba404 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4622,18 +4622,12 @@ time_t ASTWriter::getTimestampForOutput(const FileEntry 
*E) const {
 
 ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef, StringRef OutputFile,
  Module *WritingModule, StringRef isysroot,
- bool hasErrors,

[clang] Fixes and closes #53952. Setting the ASTHasCompilerErrors member variable correctly based on the PP diagnostics. (PR #68127)

2023-10-04 Thread Rajkumar Ananthu via cfe-commits 


@@ -4628,6 +4628,12 @@ ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef, 
StringRef OutputFile,
   WritingAST = true;
 
   ASTHasCompilerErrors = hasErrors;
+  bool trueHasErrors = 
SemaRef.PP.getDiagnostics().hasUncompilableErrorOccurred();

rajkumarananthu wrote:

Hi Team, Added a new commit by removing the argument `hasErrors` to the method 
`ASTWriter::WriteAST`.

https://github.com/llvm/llvm-project/pull/68127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fixes and closes #53952. Setting the ASTHasCompilerErrors member variable correctly based on the PP diagnostics. (PR #68127)

2023-10-05 Thread Rajkumar Ananthu via cfe-commits 

rajkumarananthu wrote:

Hi @AaronBallman I could see a revert of my change here: 
https://reviews.llvm.org/rGa6acf3fd49a20c570a390af2a3c84e10b9545b68

Can you please help me confirm whether the change is accepted or reverted. 
Should we resubmit the changes here?

Thanks
Rajkumar Ananthu

https://github.com/llvm/llvm-project/pull/68127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fixes and closes #53952. Setting the ASTHasCompilerErrors member variable correctly based on the PP diagnostics. (PR #68127)

2023-10-06 Thread Rajkumar Ananthu via cfe-commits 

rajkumarananthu wrote:

Thanks for taking care of this @AaronBallman 

https://github.com/llvm/llvm-project/pull/68127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fixes and closes issues, #53390 and #58710. Added new controls to IndentNamespaceAliases, IndentUsingDeclarations and DecorateReflowedComments. (PR #102894)

2024-08-12 Thread Rajkumar Ananthu via cfe-commits 

https://github.com/rajkumarananthu created 
https://github.com/llvm/llvm-project/pull/102894

Issue #53390 was reported to have an option to control indenting namespace 
aliases independently of NamespaceIndentation, added this feature.

Issue #58710 was reported to have an option to control the decoration of the 
block comments, added this feature.

>From 2bf9ae230b1b22aa1542437827749b3c19321bc2 Mon Sep 17 00:00:00 2001
From: Rajkumar Ananthu 
Date: Mon, 12 Aug 2024 18:05:10 +0530
Subject: [PATCH] Fixes and closes issues, #53390 and #58710. Added new
 controls to IndentNamespaceAliases, IndentUsingDeclarations and
 DecorateReflowedComments.

Issue #53390 was reported to have an option to control indenting
namespace aliases independently of NamespaceIndentation, added this
feature.

Issue #58710 was reported to have an option to control the decoration of
the block comments, added this feature.
---
 clang/include/clang/Format/Format.h  | 43 
 clang/lib/Format/BreakableToken.cpp  |  6 ++--
 clang/lib/Format/Format.cpp  | 16 +
 clang/lib/Format/UnwrappedLineParser.cpp | 37 ++--
 clang/lib/Format/UnwrappedLineParser.h   |  1 +
 5 files changed, 99 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index ef6c76a070bfaa..c00d57013cb13f 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2822,6 +2822,46 @@ struct FormatStyle {
   /// \version 11
   IndentExternBlockStyle IndentExternBlock;
 
+  /// IndentNamespaceAliases is the type of indenting of namespace aliases
+  /// irrespective of NamespaceIndentation.
+  bool IndentNamespaceAliases;
+
+  /// IndentUsingDeclarations is the type of indenting of using declarations
+  /// irrespective of NamespaceIndentation.
+  bool IndentUsingDeclarations;
+
+  enum DecorateReflowedCommentsStyle : int8_t {
+/// Never:
+/// don't use any decorator
+/// \code
+/// /* blah blah blah blah blah blah blah blah blah blah blah blah blah
+///blah blah blah blah blah blah blah blah */
+/// \endcode
+DRC_Never,
+/// Always:
+/// Always decorate with the decorator
+/// \code
+/// /* blah blah blah blah blah blah blah blah blah blah blah blah blah
+///  * blah blah blah blah blah blah blah blah */
+/// \endcode
+DRC_Always,
+/// FirstInLine:
+/// Use decoration only for First in line block comments
+/// \code
+/// using namespace std; /* blah blah blah blah blah blah blah blah blah
+/// blah blah blah */
+///
+/// /* blah blah blah blah blah blah blah blah blah blah blah blah blah
+///  * blah blah blah blah blah blah blah blah */
+/// using namespace std;
+/// \endcode
+DRC_FirstInLineOnly
+  };
+
+  /// reflowed block comments decoration style
+  /// \version 17
+  DecorateReflowedCommentsStyle DecorateReflowedComments;
+
   /// Options for indenting preprocessor directives.
   enum PPDirectiveIndentStyle : int8_t {
 /// Does not indent any directives.
@@ -5104,6 +5144,9 @@ struct FormatStyle {
IndentCaseBlocks == R.IndentCaseBlocks &&
IndentCaseLabels == R.IndentCaseLabels &&
IndentExternBlock == R.IndentExternBlock &&
+   IndentNamespaceAliases == R.IndentNamespaceAliases &&
+   IndentUsingDeclarations == R.IndentUsingDeclarations &&
+   DecorateReflowedComments == R.DecorateReflowedComments &&
IndentGotoLabels == R.IndentGotoLabels &&
IndentPPDirectives == R.IndentPPDirectives &&
IndentRequiresClause == R.IndentRequiresClause &&
diff --git a/clang/lib/Format/BreakableToken.cpp 
b/clang/lib/Format/BreakableToken.cpp
index 75304908dc6506..8bb18486e1717c 100644
--- a/clang/lib/Format/BreakableToken.cpp
+++ b/clang/lib/Format/BreakableToken.cpp
@@ -517,10 +517,12 @@ BreakableBlockComment::BreakableBlockComment(
   }
 
   Decoration = "* ";
-  if (Lines.size() == 1 && !FirstInLine) {
+  if ((Style.DecorateReflowedComments == FormatStyle::DRC_Never) ||
+  (!FirstInLine &&
+   (Style.DecorateReflowedComments == FormatStyle::DRC_FirstInLineOnly))) {
 // Comments for which FirstInLine is false can start on arbitrary column,
 // and available horizontal space can be too small to align consecutive
-// lines with the first one.
+// lines with the first one. Also if decoration is explicitly turned off.
 // FIXME: We could, probably, align them to current indentation level, but
 // now we just wrap them without stars.
 Decoration = "";
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 5358b35c19de25..566cbd7908ff42 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -449,6 +449,16 @@ struct 
ScalarEnumerationTraits {
   }
 };
 
+template <>
+struct ScalarEnumerationTraits {
+  static void enumeration(IO &IO

[clang] Fixes and closes issues, #53390 and #58710. Added new controls to IndentNamespaceAliases, IndentUsingDeclarations and DecorateReflowedComments. (PR #102894)

2024-08-12 Thread Rajkumar Ananthu via cfe-commits 

rajkumarananthu wrote:

I see there are some tests failing, I ran check-llvm, but I think it is the 
wrong one, I will fix them, meanwhile please proceed with the source 
code-review.

Thanks
Rajkumar Ananthu.

https://github.com/llvm/llvm-project/pull/102894
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fixes and closes issues, #53390 and #58710. Added new controls to IndentNamespaceAliases, IndentUsingDeclarations and DecorateReflowedComments. (PR #102894)

2024-08-12 Thread Rajkumar Ananthu via cfe-commits 

rajkumarananthu wrote:

Thankyou @HazardyKnusperkeks for the review, I am beginner level contributor to 
LLVM Project and that might be the reason I might have missed some minor 
details. I will address them. And as advised I will close this pull-request and 
split into multiple small reviews after addressing this suggestions.

https://github.com/llvm/llvm-project/pull/102894
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fixes and closes issues, #53390 and #58710. Added new controls to IndentNamespaceAliases, IndentUsingDeclarations and DecorateReflowedComments. (PR #102894)

2024-08-12 Thread Rajkumar Ananthu via cfe-commits 


@@ -47,8 +47,7 @@ void printLine(llvm::raw_ostream &OS, const UnwrappedLine 
&Line,
   OS << Prefix;
   NewLine = false;
 }
-OS << I->Tok->Tok.getName() << "["
-   << "T=" << (unsigned)I->Tok->getType()
+OS << I->Tok->Tok.getName() << "[" << "T=" << (unsigned)I->Tok->getType()

rajkumarananthu wrote:

I ran clang-format on each file before committing the changes, this additional 
change came in because of that.

https://github.com/llvm/llvm-project/pull/102894
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fixes and closes issues, #53390 and #58710. Added new controls to IndentNamespaceAliases, IndentUsingDeclarations and DecorateReflowedComments. (PR #102894)

2024-08-13 Thread Rajkumar Ananthu via cfe-commits 

rajkumarananthu wrote:

Hi @mydeveloperday, I will close this PR and correct it by making individual 
PRs for individual features.

Thankyou
Rajkumar.

https://github.com/llvm/llvm-project/pull/102894
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fixes and closes issues, #53390 and #58710. Added new controls to IndentNamespaceAliases, IndentUsingDeclarations and DecorateReflowedComments. (PR #102894)

2024-08-13 Thread Rajkumar Ananthu via cfe-commits 

https://github.com/rajkumarananthu closed 
https://github.com/llvm/llvm-project/pull/102894
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fixes and closes issues, #53390 and #58710. Added new controls to IndentNamespaceAliases, IndentUsingDeclarations and DecorateReflowedComments. (PR #102894)

2024-08-13 Thread Rajkumar Ananthu via cfe-commits 

rajkumarananthu wrote:

as per the conversation, closing and discarding the PR, will add individual PR 
addressing the review comments.

https://github.com/llvm/llvm-project/pull/102894
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits