Move the file public header files to `include` dir

[kaizhangNV]

Close the issue (#4635).

Move the following headers files to a include dir located at root dir of slang repo:

slang-com-helper.h -> include/slang-com-helper.h slang-com-ptr.h -> include/slang-com-ptr.h slang-gfx.h -> include/slang-gfx.h slang.h -> include/slang.h

Change cmake/SlangTarget.cmake to add include path to every target, and change the source file to use "#include " to include the public headers.

The source code update is by the script:

fileNames_slang=$(grep -r "\".*slang\.h\"" source/ -l)

for fileName in "${fileNames_slang[@]}"
do
    echo "$fileName"
    sed -i "s/\".*slang\.h\"/<slang\.h>/" $fileName
done

[jkwak-work]

I think you should still use the double-quote for the header files like

#include "slang.h"

The symbol < and > are for headers install on the system-wide, which is not the case for us. If your machine somehow has slang install on the system directory, the angled brackets wouldn't search for the local directories, which can cause an unexpected problems.

[kaizhangNV]

Change it to draft as even though it passes all the builds, it fails all the tests.

[kaizhangNV]

No user would put the "slang.h" to their project's root directory, most of use cases would add the include search path containing "slang.h", so #include <> works totally fine in that case. What you described here is that people try to build with slang without even specifying the include searching paths, I don't think that is even a practical way to integrate with any third-party software.

[jkwak-work]

No user would put the "slang.h" to their project's root directory, most of use cases would add the include search path containing "slang.h", so #include <> works totally fine in that case. What you described here is that people try to build with slang without even specifying the include searching paths, I don't think that is even a practical way to integrate with any third-party software.

My use case is when a user installed slang binaries with a package management system like apt-get. In this case, slang.h will be installed in /usr/local/include somewhere. When there are two slang.h files one in /home/jkwak/git/slang/include/slang.h, and another in /usr/local/include, the system-wide one will be used by the angled brackets.

[expipiplus1]

The symbol < and > are for headers install on the system-wide

This is not (necessarily*) correct. On all the compilers we target, "" is used for files in the current directory and <> is used for anything in the include path passed to the compiler. The "" falls back to <>, and since slang.h isn't present in the source directory they'll behave identically and any failure that happens with <> will happen in a potentially more confusing way than with "".

So it's definitely correct to use <> here.

*The C++ standard actually specifies that "" and <> methods are both implementation defined

[csyonghe]

The coding convention we've been implicitly sticking to is to use "" for all files in the slang repo, and <> for all builtin headers (std). I would prefer us sticking to that convention here and use "" for the slang headers.

[csyonghe]

The Google coding convention also requires that all include directives to use full paths from the repository root. If we ever going to clean up our code following this guideline in the future, we would also be using "" for all includes.

[kaizhangNV]

The symbol < and > are for headers install on the system-wide

This is not (necessarily*) correct. On all the compilers we target, "" is used for files in the current directory and <> is used for anything in the include path passed to the compiler. The "" falls back to <>, and since slang.h isn't present in the source directory they'll behave identically and any failure that happens with <> will happen in a potentially more confusing way than with "".

So it's definitely correct to use <> here.

*The C++ standard actually specifies that "" and <> methods are both implementation defined

I agree.

The fundamental reason that <> can be used for headers installed system-wide is because those system include search path are added to compile by default. So if you don't specify include path, those paths will be searched in higher priority.

But that is not the reason we should choose "" instead of <>, when there is multiple headers exists, we should always specify the search path to help compiler to identify the search priority.

Also, from the convention perspective, using <> can tell readers better that this header could not be located in the source directory.

[kaizhangNV]

The Google coding convention also requires that all include directives to use full paths from the repository root. If we ever going to clean up our code following this guideline in the future, we would also be using "" for all includes.

I'm not sure if we're going to follow this guideline.

But this change definitely doesn't follow this. The reason I added the include path in the cmake is because I don't want to include any path for those published headers, So they're easily handled by script.

[jkwak-work]

I still think that it is a mistake to use the angled brackets for including slang.h.

If I understood correctly, both Kai and Elli said that we should use the angled brackets because slang.h will no longer be in the current directory.

But from the readability perspective, I would expect any includes with the angled brackets to be a header file that is external from the current project; and more of system-wide headers.

[csyonghe]

since we never used <> in the codebase for anything but std, I'd say let's stay with the "" style unless there are strong counter arguments. Right now I am not seeing any so let's not change the style.

[kaizhangNV]

I will address Elli's comments and change <> to "" to make the style consistent with the other source files.

[kaizhangNV]

Please also remove things like INCLUDE_DIRECTORIES_PUBLIC ${slang_SOURCE_DIR} on any targets if they don't need to include things like this from the root dir.

You will also need to change the PUBLIC_HEADERS arg for gfx and slang to make sure they're installed correctly

@expipiplus1 can you take a look if this is correct in the latest change?

[expipiplus1]

since we never used <> in the codebase for anything but std, I'd say let's stay with the "" style unless there are strong counter arguments

This is definitely a good enough reason to use ""