[PATCH] D67025: Add .inl as valid C++ header type

[Wasim Abbas via Phabricator via cfe-commits]

abbaswasim added a comment.
Herald added a subscriber: usaxena95.

Hi All,

I am not fully aware of the process here could anyone point me to what is the 
next step? Has anyone looked at this Patch?

//Wasim


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67025



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

[PATCH] D67025: Add .inl as valid C++ header type

[Wasim Abbas via Phabricator via cfe-commits]

abbaswasim added a comment.

> FWIW, I did a search over github repos and this file extension is used, but I 
> would hardly call it a common practice

Fair enough. what would be sufficient evidence?

I think in general whether its `.inl` or any other extension thats used to 
separate template implementations out of a header needs a solution. I have 
started using `.hh` as a workaround but it not being a complete translation 
unit is still a problem. And clangd also complains about recursive includes in 
the .hh file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67025

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

[PATCH] D67025: Add .inl as valid C++ header type

[Wasim Abbas via Phabricator via cfe-commits]

abbaswasim added a comment.

Also I couldn't make the `-x` option to work with `lsp-mode` but that's not 
`clangd`s problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67025

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

[PATCH] D67025: Add .inl as valid C++ header type

[Wasim Abbas via Phabricator via cfe-commits]

abbaswasim added a comment.

AFAICT Xcode understands and reads it as c++ header file.  It highlights it 
fine and icon is "h+".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67025

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

[PATCH] D67025: Add .inl as valid C++ header type

[Wasim Abbas via Phabricator via cfe-commits]

abbaswasim created this revision.
Herald added subscribers: cfe-commits, kadircet, ilya-biryukov.
Herald added a project: clang.

`clangd` doesn't consider `.inl` a valid C++ source 
(https://github.com/clangd/clangd/issues/16#issuecomment-512942589) this pull 
request adds support for that.

Until the `export` keyword is supported by compilers mentioned 
https://stackoverflow.com/questions/5416872/using-export-keyword-with-templates 
a common way of keeping header files cleaner is to move template definitions 
into `.inl` files. This seems to be a common practice. Some evidence of it 
being commonly used for this purpose.

https://www.randygaul.net/2015/01/31/c-keyword-inline-and-inl-files/
https://drake.mit.edu/cxx_inl.html
https://www.codeproject.com/Articles/48575/How-to-define-a-template-class-in-a-h-file-and-imp
https://stackoverflow.com/questions/31949731/very-confused-on-template-inl-file
even brdf-loader https://github.com/rgl-epfl/brdf-loader

Some other ways of separating definitions is 
https://github.com/RobotLocomotion/drake/issues/3141 where they say you should 
use .inl or .impl or -inl.h or -impl.h and this might be ok but then things 
like projectile can't find it because that only works with extensions.

One caveat of this is, the above methods of using `.inl` files means its not a 
complete translation unit so `clangd` still can't fully compile it so one has 
to add `#include "header.hpp"` at the top of `.inl` files to make it work but 
thats not a big deal.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67025

Files:
  clang/lib/Driver/Types.cpp


Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -237,6 +237,7 @@
.Case("cp", TY_CXX)
.Case("cu", TY_CUDA)
.Case("hh", TY_CXXHeader)
+   .Case("inl", TY_CXXHeader)
.Case("ii", TY_PP_CXX)
.Case("ll", TY_LLVM_IR)
.Case("mi", TY_PP_ObjC)


Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -237,6 +237,7 @@
.Case("cp", TY_CXX)
.Case("cu", TY_CUDA)
.Case("hh", TY_CXXHeader)
+   .Case("inl", TY_CXXHeader)
.Case("ii", TY_PP_CXX)
.Case("ll", TY_LLVM_IR)
.Case("mi", TY_PP_ObjC)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits