[PATCH] D125258: [docs][pp-trace] Remove FileNotFound callback

[Tommy Chiang via Phabricator via cfe-commits]

oToToT created this revision.
oToToT added a reviewer: jansvoboda11.
Herald added a project: All.
oToToT requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

`FileNotFound` preprocessor callback is removed in D119708 
.
We should also remove it from the documentation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125258

Files:
  clang-tools-extra/docs/pp-trace.rst


Index: clang-tools-extra/docs/pp-trace.rst
===
--- clang-tools-extra/docs/pp-trace.rst
+++ clang-tools-extra/docs/pp-trace.rst
@@ -49,7 +49,6 @@
 
   * FileChanged
   * FileSkipped
-  * FileNotFound
   * InclusionDirective
   * moduleImport
   * EndOfMainFile
@@ -200,26 +199,6 @@
 FilenameTok: "filename.h"
 FileType: C_User
 
-`FileNotFound 
`_
 Callback
-^^
-
-FileNotFound is called when an inclusion directive results in a file-not-found 
error.
-
-Argument descriptions:
-
-==   ==   
== 
=
-Argument NameArgument Value SyntaxClang 
C++ Type Description
-==   ==   
== 
=
-FileName "(file)" 
StringRef  The name of the file being included, as written 
in the source code.
-RecoveryPath (path)   
SmallVectorImpl  If this client indicates that it can recover 
from this missing file, the client should set this as an additional header 
search patch.
-==   ==   
== 
=
-
-Example:::
-
-  - Callback: FileNotFound
-FileName: "/path/filename.h"
-RecoveryPath:
-
 `InclusionDirective 
`_
 Callback
 

 


Index: clang-tools-extra/docs/pp-trace.rst
===
--- clang-tools-extra/docs/pp-trace.rst
+++ clang-tools-extra/docs/pp-trace.rst
@@ -49,7 +49,6 @@
 
   * FileChanged
   * FileSkipped
-  * FileNotFound
   * InclusionDirective
   * moduleImport
   * EndOfMainFile
@@ -200,26 +199,6 @@
 FilenameTok: "filename.h"
 FileType: C_User
 
-`FileNotFound `_ Callback
-^^
-
-FileNotFound is called when an inclusion directive results in a file-not-found error.
-
-Argument descriptions:
-
-==   ==   == =
-Argument NameArgument Value SyntaxClang C++ Type Description
-==   ==   == =
-FileName "(file)" StringRef  The name of the file being included, as written in the source code.
-RecoveryPath (path)   SmallVectorImpl  If this client indicates that it can recover from this missing file, the client should set this as an additional header search patch.
-==   ==   == =
-
-Example:::
-
-  - Callback: FileNotFound
-FileName: "/path/filename.h"
-RecoveryPath:
-
 `InclusionDirective 

[PATCH] D125258: [docs][pp-trace] Remove FileNotFound callback

[Tommy Chiang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGba7b6f46b37b: [docs][pp-trace] Remove FileNotFound callback 
(authored by oToToT).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125258/new/

https://reviews.llvm.org/D125258

Files:
  clang-tools-extra/docs/pp-trace.rst


Index: clang-tools-extra/docs/pp-trace.rst
===
--- clang-tools-extra/docs/pp-trace.rst
+++ clang-tools-extra/docs/pp-trace.rst
@@ -49,7 +49,6 @@
 
   * FileChanged
   * FileSkipped
-  * FileNotFound
   * InclusionDirective
   * moduleImport
   * EndOfMainFile
@@ -200,26 +199,6 @@
 FilenameTok: "filename.h"
 FileType: C_User
 
-`FileNotFound 
`_
 Callback
-^^
-
-FileNotFound is called when an inclusion directive results in a file-not-found 
error.
-
-Argument descriptions:
-
-==   ==   
== 
=
-Argument NameArgument Value SyntaxClang 
C++ Type Description
-==   ==   
== 
=
-FileName "(file)" 
StringRef  The name of the file being included, as written 
in the source code.
-RecoveryPath (path)   
SmallVectorImpl  If this client indicates that it can recover 
from this missing file, the client should set this as an additional header 
search patch.
-==   ==   
== 
=
-
-Example:::
-
-  - Callback: FileNotFound
-FileName: "/path/filename.h"
-RecoveryPath:
-
 `InclusionDirective 
`_
 Callback
 

 


Index: clang-tools-extra/docs/pp-trace.rst
===
--- clang-tools-extra/docs/pp-trace.rst
+++ clang-tools-extra/docs/pp-trace.rst
@@ -49,7 +49,6 @@
 
   * FileChanged
   * FileSkipped
-  * FileNotFound
   * InclusionDirective
   * moduleImport
   * EndOfMainFile
@@ -200,26 +199,6 @@
 FilenameTok: "filename.h"
 FileType: C_User
 
-`FileNotFound `_ Callback
-^^
-
-FileNotFound is called when an inclusion directive results in a file-not-found error.
-
-Argument descriptions:
-
-==   ==   == =
-Argument NameArgument Value SyntaxClang C++ Type Description
-==   ==   == =
-FileName "(file)" StringRef  The name of the file being included, as written in the source code.
-RecoveryPath (path)   SmallVectorImpl  If this client indicates that it can recover from this missing file, the client should set this as an additional header search patch.
-==   ==   == =
-
-Example:::
-
-  - Callback: FileNotFound
-FileName: "/path/filename.h"
-RecoveryPath:
-
 `InclusionDirective `_ Callback
 ^^

[PATCH] D125373: [pp-trace] Print HashLoc in InclusionDirective callback

[Tommy Chiang via Phabricator via cfe-commits]

oToToT created this revision.
oToToT added reviewers: jtsoftware, klimek.
Herald added subscribers: kbarton, nemanjai.
Herald added a project: All.
oToToT requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The HashLoc in InclusionDirective callback is an unused parameter.
Since pp-trace is also used as a test of Clang’s PPCallbacks interface,
add it to the output of pp-trace could avoid some unintended change on
it.

This shuold resolves PR52673


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125373

Files:
  clang-tools-extra/docs/pp-trace.rst
  clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
  clang-tools-extra/test/pp-trace/pp-trace-include.cpp


Index: clang-tools-extra/test/pp-trace/pp-trace-include.cpp
===
--- clang-tools-extra/test/pp-trace/pp-trace-include.cpp
+++ clang-tools-extra/test/pp-trace/pp-trace-include.cpp
@@ -51,6 +51,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (getFileEntryForID failed)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:3:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -65,6 +66,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1A.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -95,6 +97,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1A.h"
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:4:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1B.h"
 // CHECK-NEXT:   IsAngled: false
@@ -109,6 +112,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1B.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2B.h"
 // CHECK-NEXT:   IsAngled: false
Index: clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
===
--- clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
+++ clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
@@ -137,6 +137,7 @@
 llvm::StringRef SearchPath, llvm::StringRef RelativePath,
 const Module *Imported, SrcMgr::CharacteristicKind FileType) {
   beginCallback("InclusionDirective");
+  appendArgument("HashLoc", HashLoc);
   appendArgument("IncludeTok", IncludeTok);
   appendFilePathArgument("FileName", FileName);
   appendArgument("IsAngled", IsAngled);
Index: clang-tools-extra/docs/pp-trace.rst
===
--- clang-tools-extra/docs/pp-trace.rst
+++ clang-tools-extra/docs/pp-trace.rst
@@ -223,6 +223,7 @@
 Example:::
 
   - Callback: InclusionDirective
+HashLoc: 
"D:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:4:1"
 IncludeTok: include
 FileName: "Input/Level1B.h"
 IsAngled: false


Index: clang-tools-extra/test/pp-trace/pp-trace-include.cpp
===
--- clang-tools-extra/test/pp-trace/pp-trace-include.cpp
+++ clang-tools-extra/test/pp-trace/pp-trace-include.cpp
@@ -51,6 +51,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (getFileEntryForID failed)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:3:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -65,6 +66,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1A.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -95,6 +97,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1A.h"
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:4:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1B.h"
 // CHECK-NEXT:   IsAngled: false
@@ -109,6 +112,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1B.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2B.h"
 // CHECK-NEXT:   IsAngled: false
Index: clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
=

[PATCH] D96102: [clangd] Fix missing __syncthreads.

[Tommy Chiang via Phabricator via cfe-commits]

oToToT created this revision.
oToToT added reviewers: tra, chandlerc, jdoerfert, gtbercea, emankov, 
JDevlieghere.
Herald added subscribers: usaxena95, kadircet, arphaman.
oToToT requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

I think the symbol for __syncthreads is missing. Therefore, I've added this 
into the __clang_cuda_device_functions.h.

This also solve an issue on GitHub opened about half a year ago.
Ref: https://github.com/clangd/clangd/issues/404


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96102

Files:
  clang/lib/Headers/__clang_cuda_device_functions.h


Index: clang/lib/Headers/__clang_cuda_device_functions.h
===
--- clang/lib/Headers/__clang_cuda_device_functions.h
+++ clang/lib/Headers/__clang_cuda_device_functions.h
@@ -519,6 +519,7 @@
   return __nv_fast_sincosf(__a, __s, __c);
 }
 __DEVICE__ float __sinf(float __a) { return __nv_fast_sinf(__a); }
+__DEVICE__ int __syncthreads() { return __nvvm_bar(); }
 __DEVICE__ int __syncthreads_and(int __a) { return __nvvm_bar0_and(__a); }
 __DEVICE__ int __syncthreads_count(int __a) { return __nvvm_bar0_popc(__a); }
 __DEVICE__ int __syncthreads_or(int __a) { return __nvvm_bar0_or(__a); }


Index: clang/lib/Headers/__clang_cuda_device_functions.h
===
--- clang/lib/Headers/__clang_cuda_device_functions.h
+++ clang/lib/Headers/__clang_cuda_device_functions.h
@@ -519,6 +519,7 @@
   return __nv_fast_sincosf(__a, __s, __c);
 }
 __DEVICE__ float __sinf(float __a) { return __nv_fast_sinf(__a); }
+__DEVICE__ int __syncthreads() { return __nvvm_bar(); }
 __DEVICE__ int __syncthreads_and(int __a) { return __nvvm_bar0_and(__a); }
 __DEVICE__ int __syncthreads_count(int __a) { return __nvvm_bar0_popc(__a); }
 __DEVICE__ int __syncthreads_or(int __a) { return __nvvm_bar0_or(__a); }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96102: [clangd] Fix missing __syncthreads.

[Tommy Chiang via Phabricator via cfe-commits]

oToToT updated this revision to Diff 321652.
oToToT added a comment.

I guess __nvvm_bar_sync is the right instruction.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96102/new/

https://reviews.llvm.org/D96102

Files:
  clang/lib/Headers/__clang_cuda_device_functions.h


Index: clang/lib/Headers/__clang_cuda_device_functions.h
===
--- clang/lib/Headers/__clang_cuda_device_functions.h
+++ clang/lib/Headers/__clang_cuda_device_functions.h
@@ -519,6 +519,7 @@
   return __nv_fast_sincosf(__a, __s, __c);
 }
 __DEVICE__ float __sinf(float __a) { return __nv_fast_sinf(__a); }
+__DEVICE__ int __syncthreads() { return __nvvm_bar_sync(); }
 __DEVICE__ int __syncthreads_and(int __a) { return __nvvm_bar0_and(__a); }
 __DEVICE__ int __syncthreads_count(int __a) { return __nvvm_bar0_popc(__a); }
 __DEVICE__ int __syncthreads_or(int __a) { return __nvvm_bar0_or(__a); }


Index: clang/lib/Headers/__clang_cuda_device_functions.h
===
--- clang/lib/Headers/__clang_cuda_device_functions.h
+++ clang/lib/Headers/__clang_cuda_device_functions.h
@@ -519,6 +519,7 @@
   return __nv_fast_sincosf(__a, __s, __c);
 }
 __DEVICE__ float __sinf(float __a) { return __nv_fast_sinf(__a); }
+__DEVICE__ int __syncthreads() { return __nvvm_bar_sync(); }
 __DEVICE__ int __syncthreads_and(int __a) { return __nvvm_bar0_and(__a); }
 __DEVICE__ int __syncthreads_count(int __a) { return __nvvm_bar0_popc(__a); }
 __DEVICE__ int __syncthreads_or(int __a) { return __nvvm_bar0_or(__a); }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96102: [clangd] Fix missing __syncthreads.

[Tommy Chiang via Phabricator via cfe-commits]

oToToT abandoned this revision.
oToToT added a comment.

OK, I've found what's wrong with my fix here. I should check for the clangd 
compile mechanism with more detail to fix this problem.

Sorry for disturbance.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96102/new/

https://reviews.llvm.org/D96102

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

[PATCH] D97109: [clangd] Add support for auxiliary triple specification

[Tommy Chiang via Phabricator via cfe-commits]

oToToT created this revision.
oToToT added reviewers: jdoerfert, sammccall, ilya-biryukov.
oToToT added a project: clang-tools-extra.
Herald added subscribers: usaxena95, kadircet, arphaman.
oToToT requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, MaskRay.
Herald added a project: clang.

I was working on CUDA support for clangd, and I've found that clangd is missing 
support for auxiliary triple specification. Thus, if we try making clangd work 
with CUDA or other languages that requires auxiliary triple, it will produce 
some wrong results.

After some investigation of what clang actually did, I borrowed some code from 
`clang/lib/Frontend/CompilerInstance.cpp`  to make clangd work as well.

I don't know whether this fix is reasonable or not, but it just fix something 
looks like a problem in clangd.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97109

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp


Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -370,12 +370,33 @@
   if (!Clang->hasTarget())
 return BuildPreambleError::CouldntCreateTargetInfo;
 
+  // Correctly set AuxTarget. The code is borrowed from
+  // `CompilerInstance::ExecuteAction(FrontendAction &)` inside
+  // "clang/lib/Frontend/CompilerInstance.cpp"
+  if ((Clang->getLangOpts().CUDA || Clang->getLangOpts().OpenMPIsDevice ||
+   Clang->getLangOpts().SYCLIsDevice) &&
+  !Clang->getFrontendOpts().AuxTriple.empty()) {
+auto TO = std::make_shared();
+TO->Triple = llvm::Triple::normalize(Clang->getFrontendOpts().AuxTriple);
+if (Clang->getFrontendOpts().AuxTargetCPU)
+  TO->CPU = Clang->getFrontendOpts().AuxTargetCPU.getValue();
+if (Clang->getFrontendOpts().AuxTargetFeatures)
+  TO->FeaturesAsWritten =
+  Clang->getFrontendOpts().AuxTargetFeatures.getValue();
+TO->HostTriple = Clang->getTarget().getTriple().str();
+Clang->setAuxTarget(
+TargetInfo::CreateTargetInfo(Clang->getDiagnostics(), TO));
+  }
+
   // Inform the target of the language options.
   //
   // FIXME: We shouldn't need to do this, the target should be immutable once
   // created. This complexity should be lifted elsewhere.
   Clang->getTarget().adjust(Clang->getLangOpts());
 
+  if (auto *Aux = Clang->getAuxTarget())
+Clang->getTarget().setAuxTarget(Aux);
+
   if (Clang->getFrontendOpts().Inputs.size() != 1 ||
   Clang->getFrontendOpts().Inputs[0].getKind().getFormat() !=
   InputKind::Source ||
Index: clang-tools-extra/clangd/Compiler.cpp
===
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -123,6 +123,33 @@
   if (!Clang->hasTarget())
 return nullptr;
 
+  // Correctly set AuxTarget. The code is borrowed from
+  // `CompilerInstance::ExecuteAction(FrontendAction &)` inside
+  // "clang/lib/Frontend/CompilerInstance.cpp"
+  if ((Clang->getLangOpts().CUDA || Clang->getLangOpts().OpenMPIsDevice ||
+   Clang->getLangOpts().SYCLIsDevice) &&
+  !Clang->getFrontendOpts().AuxTriple.empty()) {
+auto TO = std::make_shared();
+TO->Triple = llvm::Triple::normalize(Clang->getFrontendOpts().AuxTriple);
+if (Clang->getFrontendOpts().AuxTargetCPU)
+  TO->CPU = Clang->getFrontendOpts().AuxTargetCPU.getValue();
+if (Clang->getFrontendOpts().AuxTargetFeatures)
+  TO->FeaturesAsWritten =
+  Clang->getFrontendOpts().AuxTargetFeatures.getValue();
+TO->HostTriple = Clang->getTarget().getTriple().str();
+Clang->setAuxTarget(
+TargetInfo::CreateTargetInfo(Clang->getDiagnostics(), TO));
+  }
+
+  // Inform the target of the language options.
+  //
+  // FIXME: We shouldn't need to do this, the target should be immutable once
+  // created. This complexity should be lifted elsewhere.
+  Clang->getTarget().adjust(Clang->getLangOpts());
+
+  if (auto *Aux = Clang->getAuxTarget())
+Clang->getTarget().setAuxTarget(Aux);
+
   // RemappedFileBuffers will handle the lifetime of the Buffer pointer,
   // release it.
   Buffer.release();


Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -370,12 +370,33 @@
   if (!Clang->hasTarget())
 return BuildPreambleError::CouldntCreateTargetInfo;
 
+  // Correctly set AuxTarget. The code is borrowed from
+  // `CompilerInstance::ExecuteAction(FrontendAction &)` inside
+  // "clang/lib/Frontend/CompilerInstance.cpp"
+  if ((Clang->getLangOpts().CUDA || Clang->getLangOpts().OpenMPIsDevice ||
+   Clang->getLangOpts().SYCLIsDevice) &&
+  !Clang->getFrontendOpts().AuxTriple.empty()) {
+auto TO = st

[PATCH] D97109: [clangd] Add support for auxiliary triple specification

[Tommy Chiang via Phabricator via cfe-commits]

oToToT updated this revision to Diff 325186.
oToToT added a comment.

re-upload diff with full context, sorry for that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97109/new/

https://reviews.llvm.org/D97109

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp


Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -370,12 +370,33 @@
   if (!Clang->hasTarget())
 return BuildPreambleError::CouldntCreateTargetInfo;
 
+  // Correctly set AuxTarget. The code is borrowed from
+  // `CompilerInstance::ExecuteAction(FrontendAction &)` inside
+  // "clang/lib/Frontend/CompilerInstance.cpp"
+  if ((Clang->getLangOpts().CUDA || Clang->getLangOpts().OpenMPIsDevice ||
+   Clang->getLangOpts().SYCLIsDevice) &&
+  !Clang->getFrontendOpts().AuxTriple.empty()) {
+auto TO = std::make_shared();
+TO->Triple = llvm::Triple::normalize(Clang->getFrontendOpts().AuxTriple);
+if (Clang->getFrontendOpts().AuxTargetCPU)
+  TO->CPU = Clang->getFrontendOpts().AuxTargetCPU.getValue();
+if (Clang->getFrontendOpts().AuxTargetFeatures)
+  TO->FeaturesAsWritten =
+  Clang->getFrontendOpts().AuxTargetFeatures.getValue();
+TO->HostTriple = Clang->getTarget().getTriple().str();
+Clang->setAuxTarget(
+TargetInfo::CreateTargetInfo(Clang->getDiagnostics(), TO));
+  }
+
   // Inform the target of the language options.
   //
   // FIXME: We shouldn't need to do this, the target should be immutable once
   // created. This complexity should be lifted elsewhere.
   Clang->getTarget().adjust(Clang->getLangOpts());
 
+  if (auto *Aux = Clang->getAuxTarget())
+Clang->getTarget().setAuxTarget(Aux);
+
   if (Clang->getFrontendOpts().Inputs.size() != 1 ||
   Clang->getFrontendOpts().Inputs[0].getKind().getFormat() !=
   InputKind::Source ||
Index: clang-tools-extra/clangd/Compiler.cpp
===
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -123,6 +123,33 @@
   if (!Clang->hasTarget())
 return nullptr;
 
+  // Correctly set AuxTarget. The code is borrowed from
+  // `CompilerInstance::ExecuteAction(FrontendAction &)` inside
+  // "clang/lib/Frontend/CompilerInstance.cpp"
+  if ((Clang->getLangOpts().CUDA || Clang->getLangOpts().OpenMPIsDevice ||
+   Clang->getLangOpts().SYCLIsDevice) &&
+  !Clang->getFrontendOpts().AuxTriple.empty()) {
+auto TO = std::make_shared();
+TO->Triple = llvm::Triple::normalize(Clang->getFrontendOpts().AuxTriple);
+if (Clang->getFrontendOpts().AuxTargetCPU)
+  TO->CPU = Clang->getFrontendOpts().AuxTargetCPU.getValue();
+if (Clang->getFrontendOpts().AuxTargetFeatures)
+  TO->FeaturesAsWritten =
+  Clang->getFrontendOpts().AuxTargetFeatures.getValue();
+TO->HostTriple = Clang->getTarget().getTriple().str();
+Clang->setAuxTarget(
+TargetInfo::CreateTargetInfo(Clang->getDiagnostics(), TO));
+  }
+
+  // Inform the target of the language options.
+  //
+  // FIXME: We shouldn't need to do this, the target should be immutable once
+  // created. This complexity should be lifted elsewhere.
+  Clang->getTarget().adjust(Clang->getLangOpts());
+
+  if (auto *Aux = Clang->getAuxTarget())
+Clang->getTarget().setAuxTarget(Aux);
+
   // RemappedFileBuffers will handle the lifetime of the Buffer pointer,
   // release it.
   Buffer.release();


Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -370,12 +370,33 @@
   if (!Clang->hasTarget())
 return BuildPreambleError::CouldntCreateTargetInfo;
 
+  // Correctly set AuxTarget. The code is borrowed from
+  // `CompilerInstance::ExecuteAction(FrontendAction &)` inside
+  // "clang/lib/Frontend/CompilerInstance.cpp"
+  if ((Clang->getLangOpts().CUDA || Clang->getLangOpts().OpenMPIsDevice ||
+   Clang->getLangOpts().SYCLIsDevice) &&
+  !Clang->getFrontendOpts().AuxTriple.empty()) {
+auto TO = std::make_shared();
+TO->Triple = llvm::Triple::normalize(Clang->getFrontendOpts().AuxTriple);
+if (Clang->getFrontendOpts().AuxTargetCPU)
+  TO->CPU = Clang->getFrontendOpts().AuxTargetCPU.getValue();
+if (Clang->getFrontendOpts().AuxTargetFeatures)
+  TO->FeaturesAsWritten =
+  Clang->getFrontendOpts().AuxTargetFeatures.getValue();
+TO->HostTriple = Clang->getTarget().getTriple().str();
+Clang->setAuxTarget(
+TargetInfo::CreateTargetInfo(Clang->getDiagnostics(), TO));
+  }
+
   // Inform the target of the language options.
   //
   // FIXME: We shouldn't need to do this, the target should be immutable once
   

[PATCH] D97109: [clangd] Add support for auxiliary triple specification

[Tommy Chiang via Phabricator via cfe-commits]

oToToT added inline comments.



Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:311
 
 llvm::ErrorOr PrecompiledPreamble::Build(
 const CompilerInvocation &Invocation,

Though this file is in clang, clangd use this function to prepare preamble in 
`Preamble.cpp`, and I think it is OK to add support for auxiliary triple here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97109/new/

https://reviews.llvm.org/D97109

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

[PATCH] D97109: [clangd] Add support for auxiliary triple specification

[Tommy Chiang via Phabricator via cfe-commits]

oToToT abandoned this revision.
oToToT added a comment.

Thanks sammccall for reviewing this and for the great idea of the whole plan.

I will work on the plan (maybe without the third one) to fix the problem 
described here.

Sorry for a poor submission here again.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97109/new/

https://reviews.llvm.org/D97109

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

[PATCH] D97493: [clang][NFC] Extract Target and AuxTarget creation in CompilerInstance to new function

[Tommy Chiang via Phabricator via cfe-commits]

oToToT created this revision.
oToToT added reviewers: sammccall, rsmith.
oToToT added a project: clang.
oToToT requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.

As @sammccall mentioned in D97109 , I've 
extract the logic of creating Target and AuxTarget into a new function called 
`createTargetAndAuxTarget`.

Since there are many similar code in clang or other related tools, 
consolidating them into a single function may help others to maintain the logic 
handling target related things.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97493

Files:
  clang/include/clang/Frontend/CompilerInstance.h
  clang/lib/Frontend/CompilerInstance.cpp

Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -97,6 +97,54 @@
 void CompilerInstance::setTarget(TargetInfo *Value) { Target = Value; }
 void CompilerInstance::setAuxTarget(TargetInfo *Value) { AuxTarget = Value; }
 
+bool CompilerInstance::createTargetAndAuxTarget() {
+  // Create the target instance.
+  setTarget(TargetInfo::CreateTargetInfo(getDiagnostics(),
+ getInvocation().TargetOpts));
+  if (!hasTarget())
+return false;
+
+  // Create TargetInfo for the other side of CUDA/OpenMP/SYCL compilation.
+  if ((getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
+   getLangOpts().SYCLIsDevice) &&
+  !getFrontendOpts().AuxTriple.empty()) {
+auto TO = std::make_shared();
+TO->Triple = llvm::Triple::normalize(getFrontendOpts().AuxTriple);
+if (getFrontendOpts().AuxTargetCPU)
+  TO->CPU = getFrontendOpts().AuxTargetCPU.getValue();
+if (getFrontendOpts().AuxTargetFeatures)
+  TO->FeaturesAsWritten = getFrontendOpts().AuxTargetFeatures.getValue();
+TO->HostTriple = getTarget().getTriple().str();
+setAuxTarget(TargetInfo::CreateTargetInfo(getDiagnostics(), TO));
+  }
+
+  if (!getTarget().hasStrictFP() && !getLangOpts().ExpStrictFP) {
+if (getLangOpts().getFPRoundingMode() !=
+llvm::RoundingMode::NearestTiesToEven) {
+  getDiagnostics().Report(diag::warn_fe_backend_unsupported_fp_rounding);
+  getLangOpts().setFPRoundingMode(llvm::RoundingMode::NearestTiesToEven);
+}
+if (getLangOpts().getFPExceptionMode() != LangOptions::FPE_Ignore) {
+  getDiagnostics().Report(diag::warn_fe_backend_unsupported_fp_exceptions);
+  getLangOpts().setFPExceptionMode(LangOptions::FPE_Ignore);
+}
+// FIXME: can we disable FEnvAccess?
+  }
+
+  // Inform the target of the language options.
+  / FIXME: We shouldn't need to do this, the target should be immutable once
+  // created. This complexity should be lifted elsewhere.
+  getTarget().adjust(getLangOpts());
+
+  // Adjust target options based on codegen options.
+  getTarget().adjustTargetOptions(getCodeGenOpts(), getTargetOpts());
+
+  if (auto *Aux = getAuxTarget())
+getTarget().setAuxTarget(Aux);
+
+  return true;
+}
+
 llvm::vfs::FileSystem &CompilerInstance::getVirtualFileSystem() const {
   return getFileManager().getVirtualFileSystem();
 }
@@ -878,51 +926,9 @@
   if (!Act.PrepareToExecute(*this))
 return false;
 
-  // Create the target instance.
-  setTarget(TargetInfo::CreateTargetInfo(getDiagnostics(),
- getInvocation().TargetOpts));
-  if (!hasTarget())
+  if (!createTargetAndAuxTarget())
 return false;
 
-  // Create TargetInfo for the other side of CUDA/OpenMP/SYCL compilation.
-  if ((getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
-   getLangOpts().SYCLIsDevice) &&
-  !getFrontendOpts().AuxTriple.empty()) {
-auto TO = std::make_shared();
-TO->Triple = llvm::Triple::normalize(getFrontendOpts().AuxTriple);
-if (getFrontendOpts().AuxTargetCPU)
-  TO->CPU = getFrontendOpts().AuxTargetCPU.getValue();
-if (getFrontendOpts().AuxTargetFeatures)
-  TO->FeaturesAsWritten = getFrontendOpts().AuxTargetFeatures.getValue();
-TO->HostTriple = getTarget().getTriple().str();
-setAuxTarget(TargetInfo::CreateTargetInfo(getDiagnostics(), TO));
-  }
-
-  if (!getTarget().hasStrictFP() && !getLangOpts().ExpStrictFP) {
-if (getLangOpts().getFPRoundingMode() !=
-llvm::RoundingMode::NearestTiesToEven) {
-  getDiagnostics().Report(diag::warn_fe_backend_unsupported_fp_rounding);
-  getLangOpts().setFPRoundingMode(llvm::RoundingMode::NearestTiesToEven);
-}
-if (getLangOpts().getFPExceptionMode() != LangOptions::FPE_Ignore) {
-  getDiagnostics().Report(diag::warn_fe_backend_unsupported_fp_exceptions);
-  getLangOpts().setFPExceptionMode(LangOptions::FPE_Ignore);
-}
-// FIXME: can we disable FEnvAccess?
-  }
-
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target 

[PATCH] D97493: [clang][NFC] Extract Target and AuxTarget creation in CompilerInstance to new function

[Tommy Chiang via Phabricator via cfe-commits]

oToToT updated this revision to Diff 326581.
oToToT added a comment.

Fix typo and rename function to `createTarget`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97493/new/

https://reviews.llvm.org/D97493

Files:
  clang/include/clang/Frontend/CompilerInstance.h
  clang/lib/Frontend/CompilerInstance.cpp

Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -97,6 +97,54 @@
 void CompilerInstance::setTarget(TargetInfo *Value) { Target = Value; }
 void CompilerInstance::setAuxTarget(TargetInfo *Value) { AuxTarget = Value; }
 
+bool CompilerInstance::createTarget() {
+  // Create the target instance.
+  setTarget(TargetInfo::CreateTargetInfo(getDiagnostics(),
+ getInvocation().TargetOpts));
+  if (!hasTarget())
+return false;
+
+  // Create TargetInfo for the other side of CUDA/OpenMP/SYCL compilation.
+  if ((getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
+   getLangOpts().SYCLIsDevice) &&
+  !getFrontendOpts().AuxTriple.empty()) {
+auto TO = std::make_shared();
+TO->Triple = llvm::Triple::normalize(getFrontendOpts().AuxTriple);
+if (getFrontendOpts().AuxTargetCPU)
+  TO->CPU = getFrontendOpts().AuxTargetCPU.getValue();
+if (getFrontendOpts().AuxTargetFeatures)
+  TO->FeaturesAsWritten = getFrontendOpts().AuxTargetFeatures.getValue();
+TO->HostTriple = getTarget().getTriple().str();
+setAuxTarget(TargetInfo::CreateTargetInfo(getDiagnostics(), TO));
+  }
+
+  if (!getTarget().hasStrictFP() && !getLangOpts().ExpStrictFP) {
+if (getLangOpts().getFPRoundingMode() !=
+llvm::RoundingMode::NearestTiesToEven) {
+  getDiagnostics().Report(diag::warn_fe_backend_unsupported_fp_rounding);
+  getLangOpts().setFPRoundingMode(llvm::RoundingMode::NearestTiesToEven);
+}
+if (getLangOpts().getFPExceptionMode() != LangOptions::FPE_Ignore) {
+  getDiagnostics().Report(diag::warn_fe_backend_unsupported_fp_exceptions);
+  getLangOpts().setFPExceptionMode(LangOptions::FPE_Ignore);
+}
+// FIXME: can we disable FEnvAccess?
+  }
+
+  // Inform the target of the language options.
+  // FIXME: We shouldn't need to do this, the target should be immutable once
+  // created. This complexity should be lifted elsewhere.
+  getTarget().adjust(getLangOpts());
+
+  // Adjust target options based on codegen options.
+  getTarget().adjustTargetOptions(getCodeGenOpts(), getTargetOpts());
+
+  if (auto *Aux = getAuxTarget())
+getTarget().setAuxTarget(Aux);
+
+  return true;
+}
+
 llvm::vfs::FileSystem &CompilerInstance::getVirtualFileSystem() const {
   return getFileManager().getVirtualFileSystem();
 }
@@ -878,51 +926,9 @@
   if (!Act.PrepareToExecute(*this))
 return false;
 
-  // Create the target instance.
-  setTarget(TargetInfo::CreateTargetInfo(getDiagnostics(),
- getInvocation().TargetOpts));
-  if (!hasTarget())
+  if (!createTarget())
 return false;
 
-  // Create TargetInfo for the other side of CUDA/OpenMP/SYCL compilation.
-  if ((getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
-   getLangOpts().SYCLIsDevice) &&
-  !getFrontendOpts().AuxTriple.empty()) {
-auto TO = std::make_shared();
-TO->Triple = llvm::Triple::normalize(getFrontendOpts().AuxTriple);
-if (getFrontendOpts().AuxTargetCPU)
-  TO->CPU = getFrontendOpts().AuxTargetCPU.getValue();
-if (getFrontendOpts().AuxTargetFeatures)
-  TO->FeaturesAsWritten = getFrontendOpts().AuxTargetFeatures.getValue();
-TO->HostTriple = getTarget().getTriple().str();
-setAuxTarget(TargetInfo::CreateTargetInfo(getDiagnostics(), TO));
-  }
-
-  if (!getTarget().hasStrictFP() && !getLangOpts().ExpStrictFP) {
-if (getLangOpts().getFPRoundingMode() !=
-llvm::RoundingMode::NearestTiesToEven) {
-  getDiagnostics().Report(diag::warn_fe_backend_unsupported_fp_rounding);
-  getLangOpts().setFPRoundingMode(llvm::RoundingMode::NearestTiesToEven);
-}
-if (getLangOpts().getFPExceptionMode() != LangOptions::FPE_Ignore) {
-  getDiagnostics().Report(diag::warn_fe_backend_unsupported_fp_exceptions);
-  getLangOpts().setFPExceptionMode(LangOptions::FPE_Ignore);
-}
-// FIXME: can we disable FEnvAccess?
-  }
-
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  getTarget().adjust(getLangOpts());
-
-  // Adjust target options based on codegen options.
-  getTarget().adjustTargetOptions(getCodeGenOpts(), getTargetOpts());
-
-  if (auto *Aux = getAuxTarget())
-getTarget().setAuxTarget(Aux);
-
   // rewriter project will change target built-in bool type from its default.
   if (getFrontendOpts().Program

[PATCH] D97493: [clang][NFC] Extract Target and AuxTarget creation in CompilerInstance to new function

[Tommy Chiang via Phabricator via cfe-commits]

oToToT added a comment.

Since I don't have commit access currently, might @sammccall commit this for me 
with `--author="Yu-Hsun Chiang "`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97493/new/

https://reviews.llvm.org/D97493

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

[PATCH] D97561: [clang] Use CompilerInstance::createTarget to createTarget

[Tommy Chiang via Phabricator via cfe-commits]

oToToT created this revision.
oToToT added reviewers: sammccall, rsmith, klimek, dexonsmith.
oToToT added a project: clang.
oToToT requested review of this revision.
Herald added a subscriber: cfe-commits.

As proposed in D97109 , I tried to replace all 
similar target creating logics to a single CompilerInstance::createTarget call 
introduced in  D97493 .

This improve the consistency while creating Target.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97561

Files:
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/ChainedIncludesSource.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp
  clang/tools/clang-import-test/clang-import-test.cpp

Index: clang/tools/clang-import-test/clang-import-test.cpp
===
--- clang/tools/clang-import-test/clang-import-test.cpp
+++ clang/tools/clang-import-test/clang-import-test.cpp
@@ -205,10 +205,7 @@
 
   Ins->setInvocation(std::move(Inv));
 
-  TargetInfo *TI = TargetInfo::CreateTargetInfo(
-  Ins->getDiagnostics(), Ins->getInvocation().TargetOpts);
-  Ins->setTarget(TI);
-  Ins->getTarget().adjust(Ins->getLangOpts());
+  Ins->createTarget();
   Ins->createFileManager();
   Ins->createSourceManager(Ins->getFileManager());
   Ins->createPreprocessor(TU_Complete);
Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -365,17 +365,9 @@
   Clang->setDiagnostics(&Diagnostics);
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return BuildPreambleError::CouldntCreateTargetInfo;
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   if (Clang->getFrontendOpts().Inputs.size() != 1 ||
   Clang->getFrontendOpts().Inputs[0].getKind().getFormat() !=
   InputKind::Source ||
Index: clang/lib/Frontend/ChainedIncludesSource.cpp
===
--- clang/lib/Frontend/ChainedIncludesSource.cpp
+++ clang/lib/Frontend/ChainedIncludesSource.cpp
@@ -149,8 +149,7 @@
 new CompilerInstance(CI.getPCHContainerOperations()));
 Clang->setInvocation(std::move(CInvok));
 Clang->setDiagnostics(Diags.get());
-Clang->setTarget(TargetInfo::CreateTargetInfo(
-Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
+Clang->createTarget();
 Clang->createFileManager();
 Clang->createSourceManager(Clang->getFileManager());
 Clang->createPreprocessor(TU_Prefix);
Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1150,17 +1150,9 @@
   Clang->setDiagnostics(&getDiagnostics());
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return true;
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   assert(Clang->getFrontendOpts().Inputs.size() == 1 &&
  "Invocation must have exactly one source file!");
   assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
@@ -1568,17 +1560,9 @@
   Clang->setDiagnostics(&AST->getDiagnostics());
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return nullptr;
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   assert(Clang->getFrontendOpts().Inputs.size() == 1 &&
  "Invocation must have exactly one source file!");
   assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
@@ -2194,19 +2178,11 @@
   ProcessWarningOptions(Diag, Inv.getDiagnosticOpts());
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget()) {
+  if (!Clang->createTarget()) {
 Clang->setInvocation(nullptr);
 return;
  

[PATCH] D97561: [clang] Use CompilerInstance::createTarget to createTarget

[Tommy Chiang via Phabricator via cfe-commits]

oToToT added inline comments.



Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:368
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return BuildPreambleError::CouldntCreateTargetInfo;

Changing this without other further patch might cause `clangd` results in 
Runtime Error while handling files like CUDA.
Should I also include the patch of `clangd` or other related projects into this 
patch?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97561/new/

https://reviews.llvm.org/D97561

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

[PATCH] D97561: [clang] Use CompilerInstance::createTarget to createTarget

[Tommy Chiang via Phabricator via cfe-commits]

oToToT added a comment.

Kindly ping.

Since the part I modified is quite... old (about 4 to 7 years ago?), I hope 
I've found the right person to review for this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97561/new/

https://reviews.llvm.org/D97561

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

[PATCH] D97561: [clang] Use CompilerInstance::createTarget to createTarget

[Tommy Chiang via Phabricator via cfe-commits]

oToToT abandoned this revision.
oToToT added a comment.

Since, originally, I think it is OK to submit patch just by project, but I 
agree that it's better to make patch as minimal as possible.
Thus, I will abandon this and resubmit another patch.
(I think reuploading patch and changing the whole patch logic here might not be 
great?)




Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:368
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return BuildPreambleError::CouldntCreateTargetInfo;

sammccall wrote:
> oToToT wrote:
> > Changing this without other further patch might cause `clangd` results in 
> > Runtime Error while handling files like CUDA.
> > Should I also include the patch of `clangd` or other related projects into 
> > this patch?
> What kind of runtime error can this result in and why? What's the current 
> behavior?
> 
> My guess is if this has to be consistent between preamble + main AST, then 
> you should change PrecompiledPreamble, ASTUnit, and clangd together in one 
> patch and leave others.
Yes, this is because the inconsistency between preamble and main AST. I will 
reorder my patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97561/new/

https://reviews.llvm.org/D97561

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

[PATCH] D98128: [clang][clangd] Avoid inconsistent target creation

[Tommy Chiang via Phabricator via cfe-commits]

oToToT created this revision.
oToToT added reviewers: sammccall, rsmith, ilya-biryukov.
oToToT added projects: clang, clang-tools-extra.
Herald added subscribers: usaxena95, kadircet, arphaman.
oToToT requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, MaskRay.
Herald added a reviewer: jdoerfert.

As proposed in D97109 , I tried to make target 
creation consistent in `clang` and `clangd` by replacing the original procedure 
with a single function introduced in D97493 .

This also helps `clangd` works with CUDA, OpenMP, etc.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98128

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp

Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -365,17 +365,9 @@
   Clang->setDiagnostics(&Diagnostics);
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return BuildPreambleError::CouldntCreateTargetInfo;
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   if (Clang->getFrontendOpts().Inputs.size() != 1 ||
   Clang->getFrontendOpts().Inputs[0].getKind().getFormat() !=
   InputKind::Source ||
Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1149,18 +1149,9 @@
   // otherwise be dropped.
   Clang->setDiagnostics(&getDiagnostics());
 
-  // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return true;
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   assert(Clang->getFrontendOpts().Inputs.size() == 1 &&
  "Invocation must have exactly one source file!");
   assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
@@ -1568,17 +1559,9 @@
   Clang->setDiagnostics(&AST->getDiagnostics());
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return nullptr;
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   assert(Clang->getFrontendOpts().Inputs.size() == 1 &&
  "Invocation must have exactly one source file!");
   assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
@@ -2194,19 +2177,11 @@
   ProcessWarningOptions(Diag, Inv.getDiagnosticOpts());
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget()) {
+  if (!Clang->createTarget()) {
 Clang->setInvocation(nullptr);
 return;
   }
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   assert(Clang->getFrontendOpts().Inputs.size() == 1 &&
  "Invocation must have exactly one source file!");
   assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
Index: clang-tools-extra/clangd/Compiler.cpp
===
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -118,9 +118,7 @@
 VFS = VFSWithRemapping;
   Clang->createFileManager(VFS);
 
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return nullptr;
 
   // RemappedFileBuffers will handle the lifetime of the Buffer pointer,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98128: [clang][clangd] Avoid inconsistent target creation

[Tommy Chiang via Phabricator via cfe-commits]

oToToT updated this revision to Diff 328814.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98128/new/

https://reviews.llvm.org/D98128

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp

Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -365,17 +365,9 @@
   Clang->setDiagnostics(&Diagnostics);
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return BuildPreambleError::CouldntCreateTargetInfo;
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   if (Clang->getFrontendOpts().Inputs.size() != 1 ||
   Clang->getFrontendOpts().Inputs[0].getKind().getFormat() !=
   InputKind::Source ||
Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1150,17 +1150,9 @@
   Clang->setDiagnostics(&getDiagnostics());
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return true;
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   assert(Clang->getFrontendOpts().Inputs.size() == 1 &&
  "Invocation must have exactly one source file!");
   assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
@@ -1568,17 +1560,9 @@
   Clang->setDiagnostics(&AST->getDiagnostics());
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return nullptr;
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   assert(Clang->getFrontendOpts().Inputs.size() == 1 &&
  "Invocation must have exactly one source file!");
   assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
@@ -2194,19 +2178,11 @@
   ProcessWarningOptions(Diag, Inv.getDiagnosticOpts());
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget()) {
+  if (!Clang->createTarget()) {
 Clang->setInvocation(nullptr);
 return;
   }
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   assert(Clang->getFrontendOpts().Inputs.size() == 1 &&
  "Invocation must have exactly one source file!");
   assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
Index: clang-tools-extra/clangd/Compiler.cpp
===
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -118,9 +118,7 @@
 VFS = VFSWithRemapping;
   Clang->createFileManager(VFS);
 
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return nullptr;
 
   // RemappedFileBuffers will handle the lifetime of the Buffer pointer,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98128: [clang][clangd] Avoid inconsistent target creation

[Tommy Chiang via Phabricator via cfe-commits]

oToToT added a comment.

Kindly ping.

Maybe I should add a test?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98128/new/

https://reviews.llvm.org/D98128

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

[PATCH] D125373: [pp-trace] Print HashLoc in InclusionDirective callback

[Tommy Chiang via Phabricator via cfe-commits]

oToToT added a comment.

ping :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125373/new/

https://reviews.llvm.org/D125373

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

[PATCH] D125373: [pp-trace] Print HashLoc in InclusionDirective callback

[Tommy Chiang via Phabricator via cfe-commits]

oToToT updated this revision to Diff 433722.
oToToT added a comment.

Added a release note for the change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125373/new/

https://reviews.llvm.org/D125373

Files:
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/pp-trace.rst
  clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
  clang-tools-extra/test/pp-trace/pp-trace-include.cpp


Index: clang-tools-extra/test/pp-trace/pp-trace-include.cpp
===
--- clang-tools-extra/test/pp-trace/pp-trace-include.cpp
+++ clang-tools-extra/test/pp-trace/pp-trace-include.cpp
@@ -51,6 +51,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (getFileEntryForID failed)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:3:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -65,6 +66,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1A.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -95,6 +97,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1A.h"
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:4:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1B.h"
 // CHECK-NEXT:   IsAngled: false
@@ -109,6 +112,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1B.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2B.h"
 // CHECK-NEXT:   IsAngled: false
Index: clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
===
--- clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
+++ clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
@@ -137,6 +137,7 @@
 llvm::StringRef SearchPath, llvm::StringRef RelativePath,
 const Module *Imported, SrcMgr::CharacteristicKind FileType) {
   beginCallback("InclusionDirective");
+  appendArgument("HashLoc", HashLoc);
   appendArgument("IncludeTok", IncludeTok);
   appendFilePathArgument("FileName", FileName);
   appendArgument("IsAngled", IsAngled);
Index: clang-tools-extra/docs/pp-trace.rst
===
--- clang-tools-extra/docs/pp-trace.rst
+++ clang-tools-extra/docs/pp-trace.rst
@@ -223,6 +223,7 @@
 Example:::
 
   - Callback: InclusionDirective
+HashLoc: 
"D:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:4:1"
 IncludeTok: include
 FileName: "Input/Level1B.h"
 IsAngled: false
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -206,7 +206,7 @@
 Improvements to pp-trace
 
 
-The improvements are...
+- Added `HashLoc` information to `InclusionDirective` callback output.
 
 Clang-tidy Visual Studio plugin
 ---


Index: clang-tools-extra/test/pp-trace/pp-trace-include.cpp
===
--- clang-tools-extra/test/pp-trace/pp-trace-include.cpp
+++ clang-tools-extra/test/pp-trace/pp-trace-include.cpp
@@ -51,6 +51,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (getFileEntryForID failed)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:3:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -65,6 +66,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1A.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -95,6 +97,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1A.h"
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:4:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1B.h"
 // CHECK-NEXT:   IsAngled: false
@@ -109,6 +112,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1B.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: 

[PATCH] D125373: [pp-trace] Print HashLoc in InclusionDirective callback

[Tommy Chiang via Phabricator via cfe-commits]

oToToT added a comment.

Thanks for reviewing!
I will commit it myself later. :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125373/new/

https://reviews.llvm.org/D125373

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

[PATCH] D125373: [pp-trace] Print HashLoc in InclusionDirective callback

[Tommy Chiang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8df2b1a86680: [pp-trace] Print HashLoc in InclusionDirective 
callback (authored by oToToT).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125373/new/

https://reviews.llvm.org/D125373

Files:
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/pp-trace.rst
  clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
  clang-tools-extra/test/pp-trace/pp-trace-include.cpp


Index: clang-tools-extra/test/pp-trace/pp-trace-include.cpp
===
--- clang-tools-extra/test/pp-trace/pp-trace-include.cpp
+++ clang-tools-extra/test/pp-trace/pp-trace-include.cpp
@@ -51,6 +51,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (getFileEntryForID failed)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:3:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -65,6 +66,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1A.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -95,6 +97,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1A.h"
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:4:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1B.h"
 // CHECK-NEXT:   IsAngled: false
@@ -109,6 +112,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1B.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2B.h"
 // CHECK-NEXT:   IsAngled: false
Index: clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
===
--- clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
+++ clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
@@ -137,6 +137,7 @@
 llvm::StringRef SearchPath, llvm::StringRef RelativePath,
 const Module *Imported, SrcMgr::CharacteristicKind FileType) {
   beginCallback("InclusionDirective");
+  appendArgument("HashLoc", HashLoc);
   appendArgument("IncludeTok", IncludeTok);
   appendFilePathArgument("FileName", FileName);
   appendArgument("IsAngled", IsAngled);
Index: clang-tools-extra/docs/pp-trace.rst
===
--- clang-tools-extra/docs/pp-trace.rst
+++ clang-tools-extra/docs/pp-trace.rst
@@ -223,6 +223,7 @@
 Example:::
 
   - Callback: InclusionDirective
+HashLoc: 
"D:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:4:1"
 IncludeTok: include
 FileName: "Input/Level1B.h"
 IsAngled: false
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -239,7 +239,7 @@
 Improvements to pp-trace
 
 
-The improvements are...
+- Added `HashLoc` information to `InclusionDirective` callback output.
 
 Clang-tidy Visual Studio plugin
 ---


Index: clang-tools-extra/test/pp-trace/pp-trace-include.cpp
===
--- clang-tools-extra/test/pp-trace/pp-trace-include.cpp
+++ clang-tools-extra/test/pp-trace/pp-trace-include.cpp
@@ -51,6 +51,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (getFileEntryForID failed)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:3:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -65,6 +66,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1A.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -95,6 +97,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1A.h"
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:4:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1B.h"
 // CHECK-NEXT:   IsAngled: false
@@ -109,6 +112,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Lev

[PATCH] D80464: [CUDA] Missing __syncthreads intrinsic in __clang_cuda_device_functions.h

[Tommy Chiang via Phabricator via cfe-commits]

oToToT added a comment.

I found this patch occasionally, I think D98128 
 is what you need to solve this problem.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80464/new/

https://reviews.llvm.org/D80464

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

[PATCH] D98128: [clang][clangd] Avoid inconsistent target creation

[Tommy Chiang via Phabricator via cfe-commits]

oToToT added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98128/new/

https://reviews.llvm.org/D98128

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

[PATCH] D99407: [clang][ItaniumMangle] Check SizeExpr for DependentSizedArrayType (PR49478)

[Tommy Chiang via Phabricator via cfe-commits]

oToToT created this revision.
oToToT added reviewers: rsmith, gbenyei, MaskRay, yaxunl.
oToToT added a project: clang.
oToToT requested review of this revision.
Herald added a subscriber: cfe-commits.

As `ArrayType::ArrayType` mentioned in `clang/lib/AST/Type.cpp`, a 
DependentSizedArrayType might not have size expression because it it used as 
the type of a dependent array of unknown bound with a dependent braced 
initializer.

Thus, I add a check when mangling array of that type.

This should fix https://bugs.llvm.org/show_bug.cgi?id=49478


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99407

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/test/AST/ast-dump-array-json.cpp


Index: clang/test/AST/ast-dump-array-json.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-array-json.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ast-dump=json 
-ast-dump-filter Test %s | FileCheck %s
+
+template struct A { static const int Test[]; };
+template const int A::Test[] = { N... };
+
+// CHECK: "mangledName": "_ZN1A4TestE",
+// CHECK: "mangledName": "_ZN1A4TestE",
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3204,7 +3204,11 @@
 }
 void CXXNameMangler::mangleType(const DependentSizedArrayType *T) {
   Out << 'A';
-  mangleExpression(T->getSizeExpr());
+  // A DependentSizedArrayType might not have size expression as below
+  // 
+  //   template int arr[] = {N...};
+  if (T->getSizeExpr())
+mangleExpression(T->getSizeExpr());
   Out << '_';
   mangleType(T->getElementType());
 }


Index: clang/test/AST/ast-dump-array-json.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-array-json.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ast-dump=json -ast-dump-filter Test %s | FileCheck %s
+
+template struct A { static const int Test[]; };
+template const int A::Test[] = { N... };
+
+// CHECK: "mangledName": "_ZN1A4TestE",
+// CHECK: "mangledName": "_ZN1A4TestE",
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3204,7 +3204,11 @@
 }
 void CXXNameMangler::mangleType(const DependentSizedArrayType *T) {
   Out << 'A';
-  mangleExpression(T->getSizeExpr());
+  // A DependentSizedArrayType might not have size expression as below
+  // 
+  //   template int arr[] = {N...};
+  if (T->getSizeExpr())
+mangleExpression(T->getSizeExpr());
   Out << '_';
   mangleType(T->getElementType());
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99407: [clang][ItaniumMangle] Check SizeExpr for DependentSizedArrayType (PR49478)

[Tommy Chiang via Phabricator via cfe-commits]

oToToT updated this revision to Diff 333526.
oToToT added a comment.

Remove redundant whitespace in comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99407/new/

https://reviews.llvm.org/D99407

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/test/AST/ast-dump-array-json.cpp


Index: clang/test/AST/ast-dump-array-json.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-array-json.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ast-dump=json 
-ast-dump-filter Test %s | FileCheck %s
+
+template struct A { static const int Test[]; };
+template const int A::Test[] = { N... };
+
+// CHECK: "mangledName": "_ZN1A4TestE",
+// CHECK: "mangledName": "_ZN1A4TestE",
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3204,7 +3204,11 @@
 }
 void CXXNameMangler::mangleType(const DependentSizedArrayType *T) {
   Out << 'A';
-  mangleExpression(T->getSizeExpr());
+  // A DependentSizedArrayType might not have the size expression as below.
+  //
+  //   template int arr[] = {N...};
+  if (T->getSizeExpr())
+mangleExpression(T->getSizeExpr());
   Out << '_';
   mangleType(T->getElementType());
 }


Index: clang/test/AST/ast-dump-array-json.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-array-json.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ast-dump=json -ast-dump-filter Test %s | FileCheck %s
+
+template struct A { static const int Test[]; };
+template const int A::Test[] = { N... };
+
+// CHECK: "mangledName": "_ZN1A4TestE",
+// CHECK: "mangledName": "_ZN1A4TestE",
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3204,7 +3204,11 @@
 }
 void CXXNameMangler::mangleType(const DependentSizedArrayType *T) {
   Out << 'A';
-  mangleExpression(T->getSizeExpr());
+  // A DependentSizedArrayType might not have the size expression as below.
+  //
+  //   template int arr[] = {N...};
+  if (T->getSizeExpr())
+mangleExpression(T->getSizeExpr());
   Out << '_';
   mangleType(T->getElementType());
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99407: [clang][ItaniumMangle] Check SizeExpr for DependentSizedArrayType (PR49478)

[Tommy Chiang via Phabricator via cfe-commits]

oToToT added inline comments.



Comment at: clang/test/AST/ast-dump-array-json.cpp:6
+
+// CHECK: "mangledName": "_ZN1A4TestE",
+// CHECK: "mangledName": "_ZN1A4TestE",

yaxunl wrote:
> I am not sure if this test tests the code you change since the mangled 
> variable name does not encode type.
I think this test my code. Or, at least, this test will trigger a runtime error 
in the latest clang without this patch when mangling `A::Test`.

Also, maybe I could try construct a test to mangle variable with element type 
if needed.

WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99407/new/

https://reviews.llvm.org/D99407

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

[PATCH] D99407: [clang][ItaniumMangle] Check SizeExpr for DependentSizedArrayType (PR49478)

[Tommy Chiang via Phabricator via cfe-commits]

oToToT added inline comments.



Comment at: clang/test/AST/ast-dump-array-json.cpp:6
+
+// CHECK: "mangledName": "_ZN1A4TestE",
+// CHECK: "mangledName": "_ZN1A4TestE",

rsmith wrote:
> oToToT wrote:
> > yaxunl wrote:
> > > I am not sure if this test tests the code you change since the mangled 
> > > variable name does not encode type.
> > I think this test my code. Or, at least, this test will trigger a runtime 
> > error in the latest clang without this patch when mangling `A::Test`.
> > 
> > Also, maybe I could try construct a test to mangle variable with element 
> > type if needed.
> > 
> > WDYT?
> I would like to see a test that verifies the `A_` mangling is produced.
Ah, you're right! After some investigation, I found that this `CHECK` in the 
test doesn't really check my patch.
This test only ensure that clang won't get runtime error when handling this 
file.

However, I can't find any clever way to construct a test like this to ensure 
the output of `CXXNameMangler::mangleType(const DependentSizedArrayType *)`. 
(It is called here only for checking ABI tags, so I guess no output will be 
displayed).

I will try replace this test with a unittest to check the output.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99407/new/

https://reviews.llvm.org/D99407

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

[PATCH] D98128: [clang][clangd] Avoid inconsistent target creation

[Tommy Chiang via Phabricator via cfe-commits]

oToToT added a comment.

ping.

After some investigation, I think it is quite hard to add tests to avoid 
inconsistency between clang and clangd. Maybe I could add some tests for CUDA, 
OpenMP if needed.

WDYT


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98128/new/

https://reviews.llvm.org/D98128

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

[PATCH] D99407: [clang][ItaniumMangle] Check SizeExpr for DependentSizedArrayType (PR49478)

[Tommy Chiang via Phabricator via cfe-commits]

oToToT updated this revision to Diff 334840.
oToToT added a comment.

Updated tests to check mangled type name.

However, I not sure whether it is proper to put the tests here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99407/new/

https://reviews.llvm.org/D99407

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/unittests/AST/DeclTest.cpp


Index: clang/unittests/AST/DeclTest.cpp
===
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -104,3 +104,37 @@
   ASSERT_TRUE(0 == MangleF.compare("\x01" "foo"));
   ASSERT_TRUE(0 == MangleG.compare("goo"));
 }
+
+TEST(Decl, MangleDependentSizedArray) {
+  StringRef Code = R"(
+template 
+int A[] = {N...};
+
+template 
+struct S {
+  T B[N];
+};
+  )";
+  auto AST =
+  tooling::buildASTFromCodeWithArgs(Code, {"-target", 
"i386-apple-darwin"});
+  ASTContext &Ctx = AST->getASTContext();
+  assert(Ctx.getTargetInfo().getDataLayout().getGlobalPrefix() &&
+ "Expected target to have a global prefix");
+  DiagnosticsEngine &Diags = AST->getDiagnostics();
+
+  const auto *DeclA =
+  selectFirst("A", match(varDecl().bind("A"), Ctx));
+  const auto *DeclB =
+  selectFirst("B", match(fieldDecl().bind("B"), Ctx));
+
+  std::string MangleA, MangleB;
+  llvm::raw_string_ostream OS_A(MangleA), OS_B(MangleB);
+  std::unique_ptr MC(
+  ItaniumMangleContext::create(Ctx, Diags));
+
+  MC->mangleTypeName(DeclA->getType(), OS_A);
+  MC->mangleTypeName(DeclB->getType(), OS_B);
+
+  ASSERT_TRUE(0 == MangleA.compare("_ZTSA_i"));
+  ASSERT_TRUE(0 == MangleB.compare("_ZTSAT0__T_"));
+}
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3204,7 +3204,11 @@
 }
 void CXXNameMangler::mangleType(const DependentSizedArrayType *T) {
   Out << 'A';
-  mangleExpression(T->getSizeExpr());
+  // A DependentSizedArrayType might not have size expression as below
+  //
+  // template int arr[] = {N...};
+  if (T->getSizeExpr())
+mangleExpression(T->getSizeExpr());
   Out << '_';
   mangleType(T->getElementType());
 }


Index: clang/unittests/AST/DeclTest.cpp
===
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -104,3 +104,37 @@
   ASSERT_TRUE(0 == MangleF.compare("\x01" "foo"));
   ASSERT_TRUE(0 == MangleG.compare("goo"));
 }
+
+TEST(Decl, MangleDependentSizedArray) {
+  StringRef Code = R"(
+template 
+int A[] = {N...};
+
+template 
+struct S {
+  T B[N];
+};
+  )";
+  auto AST =
+  tooling::buildASTFromCodeWithArgs(Code, {"-target", "i386-apple-darwin"});
+  ASTContext &Ctx = AST->getASTContext();
+  assert(Ctx.getTargetInfo().getDataLayout().getGlobalPrefix() &&
+ "Expected target to have a global prefix");
+  DiagnosticsEngine &Diags = AST->getDiagnostics();
+
+  const auto *DeclA =
+  selectFirst("A", match(varDecl().bind("A"), Ctx));
+  const auto *DeclB =
+  selectFirst("B", match(fieldDecl().bind("B"), Ctx));
+
+  std::string MangleA, MangleB;
+  llvm::raw_string_ostream OS_A(MangleA), OS_B(MangleB);
+  std::unique_ptr MC(
+  ItaniumMangleContext::create(Ctx, Diags));
+
+  MC->mangleTypeName(DeclA->getType(), OS_A);
+  MC->mangleTypeName(DeclB->getType(), OS_B);
+
+  ASSERT_TRUE(0 == MangleA.compare("_ZTSA_i"));
+  ASSERT_TRUE(0 == MangleB.compare("_ZTSAT0__T_"));
+}
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3204,7 +3204,11 @@
 }
 void CXXNameMangler::mangleType(const DependentSizedArrayType *T) {
   Out << 'A';
-  mangleExpression(T->getSizeExpr());
+  // A DependentSizedArrayType might not have size expression as below
+  //
+  // template int arr[] = {N...};
+  if (T->getSizeExpr())
+mangleExpression(T->getSizeExpr());
   Out << '_';
   mangleType(T->getElementType());
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99407: [clang][ItaniumMangle] Check SizeExpr for DependentSizedArrayType (PR49478)

[Tommy Chiang via Phabricator via cfe-commits]

oToToT updated this revision to Diff 334848.
oToToT added a comment.

Remove redundant trailing spaces.

Sorry for not checking this at the first time I submit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99407/new/

https://reviews.llvm.org/D99407

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/unittests/AST/DeclTest.cpp


Index: clang/unittests/AST/DeclTest.cpp
===
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -104,3 +104,37 @@
   ASSERT_TRUE(0 == MangleF.compare("\x01" "foo"));
   ASSERT_TRUE(0 == MangleG.compare("goo"));
 }
+
+TEST(Decl, MangleDependentSizedArray) {
+  StringRef Code = R"(
+template 
+int A[] = {N...};
+
+template 
+struct S {
+  T B[N];
+};
+  )";
+  auto AST =
+  tooling::buildASTFromCodeWithArgs(Code, {"-target", 
"i386-apple-darwin"});
+  ASTContext &Ctx = AST->getASTContext();
+  assert(Ctx.getTargetInfo().getDataLayout().getGlobalPrefix() &&
+ "Expected target to have a global prefix");
+  DiagnosticsEngine &Diags = AST->getDiagnostics();
+
+  const auto *DeclA =
+  selectFirst("A", match(varDecl().bind("A"), Ctx));
+  const auto *DeclB =
+  selectFirst("B", match(fieldDecl().bind("B"), Ctx));
+
+  std::string MangleA, MangleB;
+  llvm::raw_string_ostream OS_A(MangleA), OS_B(MangleB);
+  std::unique_ptr MC(
+  ItaniumMangleContext::create(Ctx, Diags));
+
+  MC->mangleTypeName(DeclA->getType(), OS_A);
+  MC->mangleTypeName(DeclB->getType(), OS_B);
+
+  ASSERT_TRUE(0 == MangleA.compare("_ZTSA_i"));
+  ASSERT_TRUE(0 == MangleB.compare("_ZTSAT0__T_"));
+}
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3204,7 +3204,11 @@
 }
 void CXXNameMangler::mangleType(const DependentSizedArrayType *T) {
   Out << 'A';
-  mangleExpression(T->getSizeExpr());
+  // A DependentSizedArrayType might not have size expression as below
+  //
+  // template int arr[] = {N...};
+  if (T->getSizeExpr())
+mangleExpression(T->getSizeExpr());
   Out << '_';
   mangleType(T->getElementType());
 }


Index: clang/unittests/AST/DeclTest.cpp
===
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -104,3 +104,37 @@
   ASSERT_TRUE(0 == MangleF.compare("\x01" "foo"));
   ASSERT_TRUE(0 == MangleG.compare("goo"));
 }
+
+TEST(Decl, MangleDependentSizedArray) {
+  StringRef Code = R"(
+template 
+int A[] = {N...};
+
+template 
+struct S {
+  T B[N];
+};
+  )";
+  auto AST =
+  tooling::buildASTFromCodeWithArgs(Code, {"-target", "i386-apple-darwin"});
+  ASTContext &Ctx = AST->getASTContext();
+  assert(Ctx.getTargetInfo().getDataLayout().getGlobalPrefix() &&
+ "Expected target to have a global prefix");
+  DiagnosticsEngine &Diags = AST->getDiagnostics();
+
+  const auto *DeclA =
+  selectFirst("A", match(varDecl().bind("A"), Ctx));
+  const auto *DeclB =
+  selectFirst("B", match(fieldDecl().bind("B"), Ctx));
+
+  std::string MangleA, MangleB;
+  llvm::raw_string_ostream OS_A(MangleA), OS_B(MangleB);
+  std::unique_ptr MC(
+  ItaniumMangleContext::create(Ctx, Diags));
+
+  MC->mangleTypeName(DeclA->getType(), OS_A);
+  MC->mangleTypeName(DeclB->getType(), OS_B);
+
+  ASSERT_TRUE(0 == MangleA.compare("_ZTSA_i"));
+  ASSERT_TRUE(0 == MangleB.compare("_ZTSAT0__T_"));
+}
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3204,7 +3204,11 @@
 }
 void CXXNameMangler::mangleType(const DependentSizedArrayType *T) {
   Out << 'A';
-  mangleExpression(T->getSizeExpr());
+  // A DependentSizedArrayType might not have size expression as below
+  //
+  // template int arr[] = {N...};
+  if (T->getSizeExpr())
+mangleExpression(T->getSizeExpr());
   Out << '_';
   mangleType(T->getElementType());
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99407: [clang][ItaniumMangle] Check SizeExpr for DependentSizedArrayType (PR49478)

[Tommy Chiang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa89fb29398dc: [clang][ItaniumMangle] Check SizeExpr for 
DependentSizedArrayType (authored by oToToT).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99407/new/

https://reviews.llvm.org/D99407

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/unittests/AST/DeclTest.cpp


Index: clang/unittests/AST/DeclTest.cpp
===
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -104,3 +104,37 @@
   ASSERT_TRUE(0 == MangleF.compare("\x01" "foo"));
   ASSERT_TRUE(0 == MangleG.compare("goo"));
 }
+
+TEST(Decl, MangleDependentSizedArray) {
+  StringRef Code = R"(
+template 
+int A[] = {N...};
+
+template 
+struct S {
+  T B[N];
+};
+  )";
+  auto AST =
+  tooling::buildASTFromCodeWithArgs(Code, {"-target", 
"i386-apple-darwin"});
+  ASTContext &Ctx = AST->getASTContext();
+  assert(Ctx.getTargetInfo().getDataLayout().getGlobalPrefix() &&
+ "Expected target to have a global prefix");
+  DiagnosticsEngine &Diags = AST->getDiagnostics();
+
+  const auto *DeclA =
+  selectFirst("A", match(varDecl().bind("A"), Ctx));
+  const auto *DeclB =
+  selectFirst("B", match(fieldDecl().bind("B"), Ctx));
+
+  std::string MangleA, MangleB;
+  llvm::raw_string_ostream OS_A(MangleA), OS_B(MangleB);
+  std::unique_ptr MC(
+  ItaniumMangleContext::create(Ctx, Diags));
+
+  MC->mangleTypeName(DeclA->getType(), OS_A);
+  MC->mangleTypeName(DeclB->getType(), OS_B);
+
+  ASSERT_TRUE(0 == MangleA.compare("_ZTSA_i"));
+  ASSERT_TRUE(0 == MangleB.compare("_ZTSAT0__T_"));
+}
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3204,7 +3204,11 @@
 }
 void CXXNameMangler::mangleType(const DependentSizedArrayType *T) {
   Out << 'A';
-  mangleExpression(T->getSizeExpr());
+  // A DependentSizedArrayType might not have size expression as below
+  //
+  // template int arr[] = {N...};
+  if (T->getSizeExpr())
+mangleExpression(T->getSizeExpr());
   Out << '_';
   mangleType(T->getElementType());
 }


Index: clang/unittests/AST/DeclTest.cpp
===
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -104,3 +104,37 @@
   ASSERT_TRUE(0 == MangleF.compare("\x01" "foo"));
   ASSERT_TRUE(0 == MangleG.compare("goo"));
 }
+
+TEST(Decl, MangleDependentSizedArray) {
+  StringRef Code = R"(
+template 
+int A[] = {N...};
+
+template 
+struct S {
+  T B[N];
+};
+  )";
+  auto AST =
+  tooling::buildASTFromCodeWithArgs(Code, {"-target", "i386-apple-darwin"});
+  ASTContext &Ctx = AST->getASTContext();
+  assert(Ctx.getTargetInfo().getDataLayout().getGlobalPrefix() &&
+ "Expected target to have a global prefix");
+  DiagnosticsEngine &Diags = AST->getDiagnostics();
+
+  const auto *DeclA =
+  selectFirst("A", match(varDecl().bind("A"), Ctx));
+  const auto *DeclB =
+  selectFirst("B", match(fieldDecl().bind("B"), Ctx));
+
+  std::string MangleA, MangleB;
+  llvm::raw_string_ostream OS_A(MangleA), OS_B(MangleB);
+  std::unique_ptr MC(
+  ItaniumMangleContext::create(Ctx, Diags));
+
+  MC->mangleTypeName(DeclA->getType(), OS_A);
+  MC->mangleTypeName(DeclB->getType(), OS_B);
+
+  ASSERT_TRUE(0 == MangleA.compare("_ZTSA_i"));
+  ASSERT_TRUE(0 == MangleB.compare("_ZTSAT0__T_"));
+}
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3204,7 +3204,11 @@
 }
 void CXXNameMangler::mangleType(const DependentSizedArrayType *T) {
   Out << 'A';
-  mangleExpression(T->getSizeExpr());
+  // A DependentSizedArrayType might not have size expression as below
+  //
+  // template int arr[] = {N...};
+  if (T->getSizeExpr())
+mangleExpression(T->getSizeExpr());
   Out << '_';
   mangleType(T->getElementType());
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98128: [clang][clangd] Avoid inconsistent target creation

[Tommy Chiang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG14a7296c0120: [clang][clangd] Avoid inconsistent target 
creation (authored by oToToT).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98128/new/

https://reviews.llvm.org/D98128

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp

Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -365,17 +365,9 @@
   Clang->setDiagnostics(&Diagnostics);
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return BuildPreambleError::CouldntCreateTargetInfo;
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   if (Clang->getFrontendOpts().Inputs.size() != 1 ||
   Clang->getFrontendOpts().Inputs[0].getKind().getFormat() !=
   InputKind::Source ||
Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1150,17 +1150,9 @@
   Clang->setDiagnostics(&getDiagnostics());
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return true;
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   assert(Clang->getFrontendOpts().Inputs.size() == 1 &&
  "Invocation must have exactly one source file!");
   assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
@@ -1568,17 +1560,9 @@
   Clang->setDiagnostics(&AST->getDiagnostics());
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return nullptr;
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   assert(Clang->getFrontendOpts().Inputs.size() == 1 &&
  "Invocation must have exactly one source file!");
   assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
@@ -2194,19 +2178,11 @@
   ProcessWarningOptions(Diag, Inv.getDiagnosticOpts());
 
   // Create the target instance.
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget()) {
+  if (!Clang->createTarget()) {
 Clang->setInvocation(nullptr);
 return;
   }
 
-  // Inform the target of the language options.
-  //
-  // FIXME: We shouldn't need to do this, the target should be immutable once
-  // created. This complexity should be lifted elsewhere.
-  Clang->getTarget().adjust(Clang->getLangOpts());
-
   assert(Clang->getFrontendOpts().Inputs.size() == 1 &&
  "Invocation must have exactly one source file!");
   assert(Clang->getFrontendOpts().Inputs[0].getKind().getFormat() ==
Index: clang-tools-extra/clangd/Compiler.cpp
===
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -118,9 +118,7 @@
 VFS = VFSWithRemapping;
   Clang->createFileManager(VFS);
 
-  Clang->setTarget(TargetInfo::CreateTargetInfo(
-  Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));
-  if (!Clang->hasTarget())
+  if (!Clang->createTarget())
 return nullptr;
 
   // RemappedFileBuffers will handle the lifetime of the Buffer pointer,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100024: [clang] Check AuxTarget exists when creating target in CompilerInstance.

[Tommy Chiang via Phabricator via cfe-commits]

oToToT created this revision.
oToToT added reviewers: sammccall, kadircet.
oToToT added a project: clang.
oToToT requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.

D97493  separate target creation out to a 
single function `CompilerInstance::createTarget`. However, it would overwrite 
AuxTarget even if it has been set.
As @kadircet recommended in D98128 , this 
patch check the existence of AuxTarget and not overwrite it when it has been 
set.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100024

Files:
  clang/include/clang/Frontend/CompilerInstance.h
  clang/lib/Frontend/CompilerInstance.cpp


Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -104,8 +104,10 @@
   if (!hasTarget())
 return false;
 
-  // Create TargetInfo for the other side of CUDA/OpenMP/SYCL compilation.
-  if ((getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
+  // Check whether AuxTarget exists, if not, then create TargetInfo for the
+  // other side of CUDA/OpenMP/SYCL compilation.
+  if (!hasAuxTarget() &&
+  (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
getLangOpts().SYCLIsDevice) &&
   !getFrontendOpts().AuxTriple.empty()) {
 auto TO = std::make_shared();
Index: clang/include/clang/Frontend/CompilerInstance.h
===
--- clang/include/clang/Frontend/CompilerInstance.h
+++ clang/include/clang/Frontend/CompilerInstance.h
@@ -377,6 +377,8 @@
   /// @name AuxTarget Info
   /// {
 
+  bool hasAuxTarget() const { return AuxTarget != nullptr; }
+
   TargetInfo *getAuxTarget() const { return AuxTarget.get(); }
 
   /// Replace the current AuxTarget.


Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -104,8 +104,10 @@
   if (!hasTarget())
 return false;
 
-  // Create TargetInfo for the other side of CUDA/OpenMP/SYCL compilation.
-  if ((getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
+  // Check whether AuxTarget exists, if not, then create TargetInfo for the
+  // other side of CUDA/OpenMP/SYCL compilation.
+  if (!hasAuxTarget() &&
+  (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
getLangOpts().SYCLIsDevice) &&
   !getFrontendOpts().AuxTriple.empty()) {
 auto TO = std::make_shared();
Index: clang/include/clang/Frontend/CompilerInstance.h
===
--- clang/include/clang/Frontend/CompilerInstance.h
+++ clang/include/clang/Frontend/CompilerInstance.h
@@ -377,6 +377,8 @@
   /// @name AuxTarget Info
   /// {
 
+  bool hasAuxTarget() const { return AuxTarget != nullptr; }
+
   TargetInfo *getAuxTarget() const { return AuxTarget.get(); }
 
   /// Replace the current AuxTarget.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100024: [clang] Check AuxTarget exists when creating target in CompilerInstance.

[Tommy Chiang via Phabricator via cfe-commits]

oToToT added inline comments.



Comment at: clang/lib/Frontend/CompilerInstance.cpp:109
+  // other side of CUDA/OpenMP/SYCL compilation.
+  if (!hasAuxTarget() &&
+  (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||

kadircet wrote:
> can we just do `!getAuxTarget()` and not introduce `hasAuxTarget()` ? i don't 
> think the new api in the interface provides much value.
After some thinking, I’m wondering might it be more flexible if we add an 
option to let the user determine whether overwrite AuxTarget by themselves?

Like changing the prototype to
`bool CompilerInstance::createTarget(bool OverwriteAuxTarget = true);`
and simply check `if (OverwriteAuxTarget && ...` here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100024/new/

https://reviews.llvm.org/D100024

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

[PATCH] D100024: [clang] Check AuxTarget exists when creating target in CompilerInstance.

[Tommy Chiang via Phabricator via cfe-commits]

oToToT updated this revision to Diff 335778.
oToToT added a comment.

Simply use `!getAuxTarget()` to check the existence of AuxTarget.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100024/new/

https://reviews.llvm.org/D100024

Files:
  clang/lib/Frontend/CompilerInstance.cpp


Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -104,8 +104,10 @@
   if (!hasTarget())
 return false;
 
-  // Create TargetInfo for the other side of CUDA/OpenMP/SYCL compilation.
-  if ((getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
+  // Check whether AuxTarget exists, if not, then create TargetInfo for the
+  // other side of CUDA/OpenMP/SYCL compilation.
+  if (!getAuxTarget() &&
+  (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
getLangOpts().SYCLIsDevice) &&
   !getFrontendOpts().AuxTriple.empty()) {
 auto TO = std::make_shared();


Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -104,8 +104,10 @@
   if (!hasTarget())
 return false;
 
-  // Create TargetInfo for the other side of CUDA/OpenMP/SYCL compilation.
-  if ((getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
+  // Check whether AuxTarget exists, if not, then create TargetInfo for the
+  // other side of CUDA/OpenMP/SYCL compilation.
+  if (!getAuxTarget() &&
+  (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
getLangOpts().SYCLIsDevice) &&
   !getFrontendOpts().AuxTriple.empty()) {
 auto TO = std::make_shared();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100024: [clang] Check AuxTarget exists when creating target in CompilerInstance.

[Tommy Chiang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfc1117df5b87: [clang] Check AuxTarget exists when creating 
target in CompilerInstance (authored by oToToT).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100024/new/

https://reviews.llvm.org/D100024

Files:
  clang/lib/Frontend/CompilerInstance.cpp


Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -104,8 +104,10 @@
   if (!hasTarget())
 return false;
 
-  // Create TargetInfo for the other side of CUDA/OpenMP/SYCL compilation.
-  if ((getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
+  // Check whether AuxTarget exists, if not, then create TargetInfo for the
+  // other side of CUDA/OpenMP/SYCL compilation.
+  if (!getAuxTarget() &&
+  (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
getLangOpts().SYCLIsDevice) &&
   !getFrontendOpts().AuxTriple.empty()) {
 auto TO = std::make_shared();


Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -104,8 +104,10 @@
   if (!hasTarget())
 return false;
 
-  // Create TargetInfo for the other side of CUDA/OpenMP/SYCL compilation.
-  if ((getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
+  // Check whether AuxTarget exists, if not, then create TargetInfo for the
+  // other side of CUDA/OpenMP/SYCL compilation.
+  if (!getAuxTarget() &&
+  (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice ||
getLangOpts().SYCLIsDevice) &&
   !getFrontendOpts().AuxTriple.empty()) {
 auto TO = std::make_shared();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108756: [clang] Add '-ast-dump-filter=' support

[Tommy Chiang via Phabricator via cfe-commits]

oToToT created this revision.
oToToT added reviewers: rsmith, alexfh.
oToToT added a project: clang.
Herald added a subscriber: dang.
oToToT requested review of this revision.
Herald added a subscriber: cfe-commits.

Before this patch, we only support syntax like
`clang -cc1 -ast-dump -ast-dump-filter main a.c`
or
`clang -Xclang -ast-dump -Xclang -ast-dump-filter -Xclang main a.c`
when using ast-dump-filter.

It is helpful to also support `-ast-dump-filter=` syntax, so we can do 
something like
`clang -cc1 -ast-dump -ast-dump-filter=main a.c`
or
`clang -Xclang -ast-dump -Xclang -ast-dump-filter=main a.c`

It is more cleaner when passing arguments through `-Xclang` in this case.

Also, **clang-check** do support this syntax, and I think people might be 
confused when they found they can't use `-ast-dump-filter` with clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108756

Files:
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5277,6 +5277,8 @@
" nodes having a certain substring in a qualified name. Use"
" -ast-list to list all filterable declaration node names.">,
   MarshallingInfoString>;
+def ast_dump_filter_EQ : Joined<["-"], "ast-dump-filter=">,
+  Alias;
 def fno_modules_global_index : Flag<["-"], "fno-modules-global-index">,
   HelpText<"Do not automatically generate or update the global module index">,
   MarshallingInfoNegativeFlag>;


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5277,6 +5277,8 @@
" nodes having a certain substring in a qualified name. Use"
" -ast-list to list all filterable declaration node names.">,
   MarshallingInfoString>;
+def ast_dump_filter_EQ : Joined<["-"], "ast-dump-filter=">,
+  Alias;
 def fno_modules_global_index : Flag<["-"], "fno-modules-global-index">,
   HelpText<"Do not automatically generate or update the global module index">,
   MarshallingInfoNegativeFlag>;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108756: [clang] Add '-ast-dump-filter=' support

[Tommy Chiang via Phabricator via cfe-commits]

oToToT added a comment.

ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108756/new/

https://reviews.llvm.org/D108756

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