[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

[Nathan Ridge via Phabricator via cfe-commits]

nridge planned changes to this revision.
nridge added a comment.

Ok, I can try formulating this as part of / an extension to semantic 
highlighting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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

[PATCH] D67358: [clangd] Implement semantic selections.

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Which LSP feature is this related to?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67358



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

Re: r371918 - [Driver] Fix multiple bugs related to dependency file options: -M -MM -MD -MMD -MT -MQ

[Fāng-ruì Sòng via cfe-commits]

It was related to D67542   It tried to
solve some other problems.

On Sun, Sep 15, 2019 at 1:40 AM Nico Weber  wrote:

> This looks similar to https://reviews.llvm.org/D67542
>
> On Sat, Sep 14, 2019 at 1:59 AM Fangrui Song via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: maskray
>> Date: Fri Sep 13 23:01:22 2019
>> New Revision: 371918
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=371918=rev
>> Log:
>> [Driver] Fix multiple bugs related to dependency file options: -M -MM -MD
>> -MMD -MT -MQ
>>
>> -M -o test.i => dependency file is test.d, not test.i
>> -MM -o test.i => dependency file is test.d, not test.i
>> -M -MMD => bogus warning -Wunused-command-line-argument
>> -M MT dummy => -w not rendered
>>
>> Modified:
>> cfe/trunk/lib/Driver/Driver.cpp
>> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>> cfe/trunk/test/Driver/m-and-mm.c
>>
>> Modified: cfe/trunk/lib/Driver/Driver.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=371918=371917=371918=diff
>>
>> ==
>> --- cfe/trunk/lib/Driver/Driver.cpp (original)
>> +++ cfe/trunk/lib/Driver/Driver.cpp Fri Sep 13 23:01:22 2019
>> @@ -3450,8 +3450,10 @@ Action *Driver::ConstructPhaseAction(
>>  llvm_unreachable("link action invalid here.");
>>case phases::Preprocess: {
>>  types::ID OutputTy;
>> -// -{M, MM} alter the output type.
>> -if (Args.hasArg(options::OPT_M, options::OPT_MM)) {
>> +// -M and -MM specify the dependency file name by altering the
>> output type,
>> +// -if -MD and -MMD are not specified.
>> +if (Args.hasArg(options::OPT_M, options::OPT_MM) &&
>> +!Args.hasArg(options::OPT_MD, options::OPT_MMD)) {
>>OutputTy = types::TY_Dependencies;
>>  } else {
>>OutputTy = Input->getType();
>>
>> Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=371918=371917=371918=diff
>>
>> ==
>> --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
>> +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Fri Sep 13 23:01:22 2019
>> @@ -1061,7 +1061,6 @@ void Clang::AddPreprocessingOptions(Comp
>>  ArgStringList ,
>>  const InputInfo ,
>>  const InputInfoList ) const {
>> -  Arg *A;
>>const bool IsIAMCU = getToolChain().getTriple().isOSIAMCU();
>>
>>CheckPreprocessingOptions(D, Args);
>> @@ -1070,9 +1069,20 @@ void Clang::AddPreprocessingOptions(Comp
>>Args.AddLastArg(CmdArgs, options::OPT_CC);
>>
>>// Handle dependency file generation.
>> -  if ((A = Args.getLastArg(options::OPT_M, options::OPT_MM)) ||
>> -  (A = Args.getLastArg(options::OPT_MD)) ||
>> -  (A = Args.getLastArg(options::OPT_MMD))) {
>> +  Arg *ArgM = Args.getLastArg(options::OPT_MM);
>> +  if (!ArgM)
>> +ArgM = Args.getLastArg(options::OPT_M);
>> +  Arg *ArgMD = Args.getLastArg(options::OPT_MMD);
>> +  if (!ArgMD)
>> +ArgMD = Args.getLastArg(options::OPT_MD);
>> +
>> +  // -M and -MM imply -w.
>> +  if (ArgM)
>> +CmdArgs.push_back("-w");
>> +  else
>> +ArgM = ArgMD;
>> +
>> +  if (ArgM) {
>>  // Determine the output location.
>>  const char *DepFile;
>>  if (Arg *MF = Args.getLastArg(options::OPT_MF)) {
>> @@ -1080,8 +1090,7 @@ void Clang::AddPreprocessingOptions(Comp
>>C.addFailureResultFile(DepFile, );
>>  } else if (Output.getType() == types::TY_Dependencies) {
>>DepFile = Output.getFilename();
>> -} else if (A->getOption().matches(options::OPT_M) ||
>> -   A->getOption().matches(options::OPT_MM)) {
>> +} else if (!ArgMD) {
>>DepFile = "-";
>>  } else {
>>DepFile = getDependencyFileName(Args, Inputs);
>> @@ -1090,8 +1099,22 @@ void Clang::AddPreprocessingOptions(Comp
>>  CmdArgs.push_back("-dependency-file");
>>  CmdArgs.push_back(DepFile);
>>
>> +bool HasTarget = false;
>> +for (const Arg *A : Args.filtered(options::OPT_MT, options::OPT_MQ))
>> {
>> +  HasTarget = true;
>> +  A->claim();
>> +  if (A->getOption().matches(options::OPT_MT)) {
>> +A->render(Args, CmdArgs);
>> +  } else {
>> +CmdArgs.push_back("-MT");
>> +SmallString<128> Quoted;
>> +QuoteTarget(A->getValue(), Quoted);
>> +CmdArgs.push_back(Args.MakeArgString(Quoted));
>> +  }
>> +}
>> +
>>  // Add a default target if one wasn't specified.
>> -if (!Args.hasArg(options::OPT_MT) && !Args.hasArg(options::OPT_MQ)) {
>> +if (!HasTarget) {
>>const char *DepTarget;
>>
>>// If user provided -o, that is the dependency target, except
>> @@ -1108,17 +1131,14 @@ void Clang::AddPreprocessingOptions(Comp
>>  DepTarget = 

[PATCH] D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added inline comments.



Comment at: cfe/trunk/include/clang/Basic/AttributeCommonInfo.h:166
+   ? SpellingIndex
+   : calculateAttributeSpellingListIndex();
+  }

erichkeane wrote:
> aheejin wrote:
> > MaskRay wrote:
> > > calculateAttributeSpellingListIndex is defined in clangSema. This can 
> > > cause lib/libclangAST.so.10svn (-DBUILD_SHARED_LIBS=on) fail to link:
> > > 
> > > ```
> > > ld.lld: error: undefined symbol: 
> > > clang::AttributeCommonInfo::calculateAttributeSpellingListIndex() const   
> > > 
> > > >>> referenced by AttributeCommonInfo.h:166 
> > > >>> (../tools/clang/include/clang/Basic/AttributeCommonInfo.h:166) 
> > > >>>   
> > > >>> tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/AttrImpl.cpp.o:(clang::AttributeCommonInfo::getAttributeSpellingListIndex()
> > > >>>  const)
> > > ```
> > +1 This fails builds with `-DBUILD_SHARED_LIBS=ON`. I tried to add 
> > `clangSema` as a dependent library to `clangAST`, but this creates several 
> > circular dependencies.
> Thanks for the heads up.  The solution will just end up being moving that 
> function's definition.  I'll submit a patch on monday. Thanks for the 
> reproducer.
Normally I’d suggest a revert, but I can see downstream some stuff with Swift 
and apinotes was not completely trivial to get merged in with this patch. Is 
simply moving the definition the right solution here btw?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67368



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

[PATCH] D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr.

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked an inline comment as done.
erichkeane added inline comments.



Comment at: cfe/trunk/include/clang/Basic/AttributeCommonInfo.h:166
+   ? SpellingIndex
+   : calculateAttributeSpellingListIndex();
+  }

aheejin wrote:
> MaskRay wrote:
> > calculateAttributeSpellingListIndex is defined in clangSema. This can cause 
> > lib/libclangAST.so.10svn (-DBUILD_SHARED_LIBS=on) fail to link:
> > 
> > ```
> > ld.lld: error: undefined symbol: 
> > clang::AttributeCommonInfo::calculateAttributeSpellingListIndex() const 
> >   
> > >>> referenced by AttributeCommonInfo.h:166 
> > >>> (../tools/clang/include/clang/Basic/AttributeCommonInfo.h:166) 
> > >>>   
> > >>> tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/AttrImpl.cpp.o:(clang::AttributeCommonInfo::getAttributeSpellingListIndex()
> > >>>  const)
> > ```
> +1 This fails builds with `-DBUILD_SHARED_LIBS=ON`. I tried to add 
> `clangSema` as a dependent library to `clangAST`, but this creates several 
> circular dependencies.
Thanks for the heads up.  The solution will just end up being moving that 
function's definition.  I'll submit a patch on monday. Thanks for the 
reproducer.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67368



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

[PATCH] D67253: clang-misexpect: a standalone tool for verifying the use of __builtin_expect with PGO data

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D67253#1670704 , @paulkirth wrote:

> In D67253#1670681 , @lebedev.ri 
> wrote:
>
> > Re concurrency - you had standalone `LLVMContext` for each thread, right?
>
>
> I believe there was, but this is just whatever happens when libtooling 
> creates and executes a compiler invocation. It seems to me that there is some 
> global state that ends up being shared between threads. I wasn’t able to 
> fully trace the issue, but it at least partly involves the processing of 
> backend options. I believe there is more, that needs to be addressed but this 
> is probably a good place for me to start. Thanks for the suggestion.


Ah yes, i don't think this can work given existing interface.

>> In D67253#1670667 , @paulkirth 
>> wrote:
>> 
>>> In D67253#1670569 , @lebedev.ri 
>>> wrote:
>>>
>>> > Layering feels weird here.
>>> >  Can this be implemented as/limited to just a
>>> >  `run-clang-misexpect.py` wrapper over clang itself?
>>> >  That would be best IMHO.
>>>
>>>
>>> I discussed the concurrency issue with some folks who work on libTooling, 
>>> notably Sam McCall & Dmitri Gribenko. This was the approach they suggested 
>>> I follow. LibTooling also provides some nice mechanisms for curating 
>>> compiler options, which is possible, but less than ideal to reimplement in 
>>> a python script. There are probably more benefits that escape me at the 
>>> moment, but that was the first one I thought of.
>>>
>>> Out of curiosity, if the concurrency issue was fixed in the compiler and 
>>> the python script was removed/deprecated, would you still feel the same way?
>> 
>> 
>> I was actually talking/thinking about the opposite direction, only having 
>> the .py wrapper, no standalone tool;
>>  so the opposite solution (no .py, only the tool) is tangential/has 
>> different direction.
> 
> Right, I was asking if you still see this as an issue if the python script 
> wasn’t necessary. I take from this comment that it would not make you feel 
> differently.

Right. Not really a blocker, just seems counter-general - there already is a
similar script to run clang-tidy on compilation database, and this script/tool
will have the same purpose - to just ran `clang -Wmisexpect`+profile on 
compilation database.
It really seems like some generalization is missing.

> I think ultimately keeping the implementation in c++ makes the most sense and 
> can evolve with the rest of libtooling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67253



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

[PATCH] D67253: clang-misexpect: a standalone tool for verifying the use of __builtin_expect with PGO data

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

In D67253#1670681 , @lebedev.ri wrote:

> Re concurrency - you had standalone `LLVMContext` for each thread, right?


I believe there was, but this is just whatever happens when libtooling creates 
and executes a compiler invocation. It seems to me that there is some global 
state that ends up being shared between threads. I wasn’t able to fully trace 
the issue, but it at least partly involves the processing of backend options. I 
believe there is more, that needs to be addressed but this is probably a good 
place for me to start. Thanks for the suggestion.

> 
> 
> In D67253#1670667 , @paulkirth wrote:
> 
>> In D67253#1670569 , @lebedev.ri 
>> wrote:
>>
>> > Layering feels weird here.
>> >  Can this be implemented as/limited to just a
>> >  `run-clang-misexpect.py` wrapper over clang itself?
>> >  That would be best IMHO.
>>
>>
>> I discussed the concurrency issue with some folks who work on libTooling, 
>> notably Sam McCall & Dmitri Gribenko. This was the approach they suggested I 
>> follow. LibTooling also provides some nice mechanisms for curating compiler 
>> options, which is possible, but less than ideal to reimplement in a python 
>> script. There are probably more benefits that escape me at the moment, but 
>> that was the first one I thought of.
>>
>> Out of curiosity, if the concurrency issue was fixed in the compiler and the 
>> python script was removed/deprecated, would you still feel the same way?
> 
> 
> I was actually talking/thinking about the opposite direction, only having the 
> .py wrapper, no standalone tool;
>  so the opposite solution (no .py, only the tool) is tangential/has different 
> direction.

Right, I was asking if you still see this as an issue if the python script 
wasn’t necessary. I take from this comment that it would not make you feel 
differently.

I think ultimately keeping the implementation in c++ makes the most sense and 
can evolve with the rest of libtooling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67253



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

[PATCH] D67588: Add builtin trait for add/remove cv (and similar)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

@lebedev.ri after adding _only_ these builtins to libc++ the type trait tests 
run several seconds faster. I think if we update _all_ the type traits to use 
builtins then, it could increase speed of the type trait tests by as much as 
50% (if not more).


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

https://reviews.llvm.org/D67588



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

[PATCH] D67253: clang-misexpect: a standalone tool for verifying the use of __builtin_expect with PGO data

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Re concurrency - you had standalone `LLVMContext` for each thread, right?

In D67253#1670667 , @paulkirth wrote:

> In D67253#1670569 , @lebedev.ri 
> wrote:
>
> > Layering feels weird here.
> >  Can this be implemented as/limited to just a
> >  `run-clang-misexpect.py` wrapper over clang itself?
> >  That would be best IMHO.
>
>
> I discussed the concurrency issue with some folks who work on libTooling, 
> notably Sam McCall & Dmitri Gribenko. This was the approach they suggested I 
> follow. LibTooling also provides some nice mechanisms for curating compiler 
> options, which is possible, but less than ideal to reimplement in a python 
> script. There are probably more benefits that escape me at the moment, but 
> that was the first one I thought of.
>
> Out of curiosity, if the concurrency issue was fixed in the compiler and the 
> python script was removed/deprecated, would you still feel the same way?


I was actually talking/thinking about the opposite direction, only having the 
.py wrapper, no standalone tool;
so the opposite solution (no .py, only the tool) is tangential/has different 
direction.




Comment at: clang-tools-extra/docs/clang-misexpect.rst:1-3
+===
+Clang-Misexpect
+===

I was just thinking that i forgot to mention docs in the original review, so 
thanks.

That being said, i do wonder whether this docs should be reworked
into two separate pages as it is a //bit// misleading right now.
The bulk of this should talk about
clang's `-Wmisexpect`/llvm's `-pgo-warn-misexpect`.

Because right now it seems as-if one has to use the tool,
and as it was brought up in the lists multiple times,
[depending on the exact workflow] that is more involved
than simply passing one more warning flag to the compiler.

(Yes, after specifically looking for it, i can see that
there are mentions/hints that the tool isn't required,
but it's a bit too subtle.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67253



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

[PATCH] D67253: clang-misexpect: a standalone tool for verifying the use of __builtin_expect with PGO data

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

In D67253#1670569 , @lebedev.ri wrote:

> Layering feels weird here.
>  Can this be implemented as/limited to just a
>  `run-clang-misexpect.py` wrapper over clang itself?
>  That would be best IMHO.


I discussed the concurrency issue with some folks who work on libTooling, 
notably Sam McCall & Dmitri Gribenko. This was the approach they suggested I 
follow. LibTooling also provides some nice mechanisms for curating compiler 
options, which is possible, but less than ideal to reimplement in a python 
script. There are probably more benefits that escape me at the moment, but that 
was the first one I thought of.

Out of curiosity, if the concurrency issue was fixed in the compiler and the 
python script was removed/deprecated, would you still feel the same way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67253



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

Re: [PATCH] D66564: [clang-tidy] new FPGA struct pack align check

[Alexandre Isoard via cfe-commits]

I'm not sure what is the advantage of this compared to -Wpadded?

On Sun, Sep 15, 2019 at 12:05 PM Roman Lebedev via Phabricator via
llvm-commits  wrote:

> lebedev.ri added a comment.
>
> Forgot the most important question.
> Right now this will fire on every single struct.
> But it won't matter unless the alignment/size actually matters, and most
> often that will happen when you have e.g. a vector of such structs.
> What i'm asking is - should this be more picky, and complain only about
> the cases where this matters?
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D66564/new/
>
> https://reviews.llvm.org/D66564
>
>
>
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>


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

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Forgot the most important question.
Right now this will fire on every single struct.
But it won't matter unless the alignment/size actually matters, and most often 
that will happen when you have e.g. a vector of such structs.
What i'm asking is - should this be more picky, and complain only about the 
cases where this matters?


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

https://reviews.llvm.org/D66564



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

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

I, too, don't believe this is FPGA specific; it should likely go into `misc-` 
or even `performance-`.
The wording of the diags seems weird to me, it would be good to 1. add more 
explanation to the docs and 2. reword the diags.




Comment at: clang-tidy/fpga/StructPackAlignCheck.cpp:31
+  // Get sizing info for the struct
+  std::vector> FieldSizes;
+  unsigned int TotalBitSize = 0;

`llvm::SmallVector`



Comment at: clang-tidy/fpga/StructPackAlignCheck.cpp:50
+  CharUnits CurrSize = Result.Context->getASTRecordLayout(Struct).getSize();
+  CharUnits MinByteSize = CharUnits::fromQuantity((TotalBitSize + 7) / 8);
+  CharUnits CurrAlign =

`roundUpTo()`?



Comment at: clang-tidy/fpga/StructPackAlignCheck.cpp:54-62
+  if (!MinByteSize.isPowerOfTwo()) {
+int MSB = (int)MinByteSize.getQuantity();
+for (; MSB > 0; MSB /= 2) {
+  NewAlign = NewAlign.alignTo(
+  CharUnits::fromQuantity(((int)NewAlign.getQuantity()) * 2));
+  // Abort if the computed alignment meets the maximum configured alignment
+  if (NewAlign.getQuantity() >= MAX_CONFIGURED_ALIGNMENT)

I'm not sure what is going on here.
I think this should be a helper function with meaningful name.



Comment at: clang-tidy/fpga/StructPackAlignCheck.cpp:67-76
+  // Check if struct has a "packed" attribute
+  bool IsPacked = false;
+  if (Struct->hasAttrs()) {
+for (const Attr *StructAttribute : Struct->getAttrs()) {
+  if (StructAttribute->getKind() == attr::Packed) {
+IsPacked = true;
+break;

```
bool IsPacked = Struct->hasAttr();
```



Comment at: docs/clang-tidy/checks/fpga-struct-pack-align.rst:45
+
+  // Explicitly aligning a struct to a bad value will result in a warning
+  struct badly_aligned_example {

'bad'?



Comment at: test/clang-tidy/fpga-struct-pack-align.cpp:9
+};
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: struct 'error' has inefficient 
access due to padding, only needs 10 bytes but is using 24 bytes, use 
"__attribute((packed))" [fpga-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: warning: struct 'error' has inefficient 
access due to poor alignment; currently aligned to 8 bytes, but size 10 bytes 
is large enough to benefit from "__attribute((aligned(16)))" 
[fpga-struct-pack-align]

'has inefficient access' reads weird to me. I'm not sure what that actually 
means.


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

https://reviews.llvm.org/D66564



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

[PATCH] D67592: [Clang] Use -main-file-name for source filename if not set

[Joel Klinghed via Phabricator via cfe-commits]

the_jk added a comment.

Forgot to link: https://bugs.llvm.org/show_bug.cgi?id=43250


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67592



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

[PATCH] D67592: [Clang] Use -main-file-name for source filename if not set

[Joel Klinghed via Phabricator via cfe-commits]

the_jk created this revision.
the_jk added a reviewer: rsmith.
the_jk added a project: clang.
Herald added subscribers: cfe-commits, aprantl.

-main-file-name is currently used to set the source name used in debug 
information.

If the source filename is "-" and -main-file-name is set, then use the filename 
also for source_filename and ModuleID of the output.

  

The argument is generally used outside the internal clang calls when running 
clang in a wrapper like icecc which gives the source via stdin but still wants 
to get a object file with the original source filename both in debug info and 
IR code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67592

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/lib/Frontend/FrontendAction.cpp
  clang/test/Frontend/stdin-input


Index: clang/test/Frontend/stdin-input
===
--- /dev/null
+++ clang/test/Frontend/stdin-input
@@ -0,0 +1,7 @@
+// RUN: cat %s | %clang -emit-llvm -g -S \
+// RUN: -Xclang -main-file-name -Xclang test/foo.c -x c - -o - | FileCheck %s
+// CHECK: ; ModuleID = 'test/foo.c'
+// CHECK: source_filename = "test/foo.c"
+// CHECK: !1 = !DIFile(filename: "test/foo.c"
+
+int main() {}
\ No newline at end of file
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -819,6 +819,10 @@
 std::string PresumedInputFile = getCurrentFileOrBufferName();
 if (Input.isPreprocessed())
   ReadOriginalFileName(CI, PresumedInputFile);
+else if (PresumedInputFile == "-" &&
+ !CI.getCodeGenOpts().MainFileName.empty()) {
+  PresumedInputFile = CI.getCodeGenOpts().MainFileName;
+}
 
 std::unique_ptr Consumer =
 CreateWrappedASTConsumer(CI, PresumedInputFile);
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -687,7 +687,7 @@
 def version : Flag<["-"], "version">,
   HelpText<"Print the compiler version">;
 def main_file_name : Separate<["-"], "main-file-name">,
-  HelpText<"Main file name to use for debug info">;
+  HelpText<"Main file name to use for debug info and source if missing">;
 def split_dwarf_output : Separate<["-"], "split-dwarf-output">,
   HelpText<"File name to use for split dwarf debug info output">;
 


Index: clang/test/Frontend/stdin-input
===
--- /dev/null
+++ clang/test/Frontend/stdin-input
@@ -0,0 +1,7 @@
+// RUN: cat %s | %clang -emit-llvm -g -S \
+// RUN: -Xclang -main-file-name -Xclang test/foo.c -x c - -o - | FileCheck %s
+// CHECK: ; ModuleID = 'test/foo.c'
+// CHECK: source_filename = "test/foo.c"
+// CHECK: !1 = !DIFile(filename: "test/foo.c"
+
+int main() {}
\ No newline at end of file
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -819,6 +819,10 @@
 std::string PresumedInputFile = getCurrentFileOrBufferName();
 if (Input.isPreprocessed())
   ReadOriginalFileName(CI, PresumedInputFile);
+else if (PresumedInputFile == "-" &&
+ !CI.getCodeGenOpts().MainFileName.empty()) {
+  PresumedInputFile = CI.getCodeGenOpts().MainFileName;
+}
 
 std::unique_ptr Consumer =
 CreateWrappedASTConsumer(CI, PresumedInputFile);
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -687,7 +687,7 @@
 def version : Flag<["-"], "version">,
   HelpText<"Print the compiler version">;
 def main_file_name : Separate<["-"], "main-file-name">,
-  HelpText<"Main file name to use for debug info">;
+  HelpText<"Main file name to use for debug info and source if missing">;
 def split_dwarf_output : Separate<["-"], "split-dwarf-output">,
   HelpText<"File name to use for split dwarf debug info output">;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies marked 22 inline comments as done.
ffrankies added a comment.

In D66564#1647779 , @BlackAngel35 
wrote:

> > Please mention new module and check in Release Notes.


The new module and check are now mentioned in Release Notes.

P.S. The above change was requested by a user that has since been disabled. Is 
there something I have to do about this?


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

https://reviews.llvm.org/D66564



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

r371934 - [OpenMP] Fix OMPClauseReader::readClause() uninitialized variable warning. NFCI.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Sun Sep 15 09:05:20 2019
New Revision: 371934

URL: http://llvm.org/viewvc/llvm-project?rev=371934=rev
Log:
[OpenMP] Fix OMPClauseReader::readClause() uninitialized variable warning. NFCI.

Fixes static analyzer uninitialized variable warning for the OMPClause - the 
function appears to cover all cases, but I've added an assertion to make sure.

Modified:
cfe/trunk/lib/Serialization/ASTReader.cpp

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=371934=371933=371934=diff
==
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Sun Sep 15 09:05:20 2019
@@ -12202,7 +12202,7 @@ Expected ASTRecordReader::read
 
===--===//
 
 OMPClause *OMPClauseReader::readClause() {
-  OMPClause *C;
+  OMPClause *C = nullptr;
   switch (Record.readInt()) {
   case OMPC_if:
 C = new (Context) OMPIfClause();
@@ -12403,6 +12403,8 @@ OMPClause *OMPClauseReader::readClause()
 C = OMPAllocateClause::CreateEmpty(Context, Record.readInt());
 break;
   }
+  assert(C && "Unknown OMPClause type");
+
   Visit(C);
   C->setLocStart(Record.readSourceLocation());
   C->setLocEnd(Record.readSourceLocation());


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

[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: clang/include/clang/Analysis/PathDiagnostic.h:81
+  /// because deterministic mode is always superior when done right, but
+  /// for some consumers is experimental and needs to be off by default.
+  bool ShouldWriteStableReportFilename;

gribozavr wrote:
> "but for some consumers is experimental" -- parse error. What is experimental?
I suspect you copied these from the `.def` file, did you change the 
descriptions there too?


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

https://reviews.llvm.org/D67420



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

[PATCH] D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr.

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added inline comments.



Comment at: cfe/trunk/include/clang/Basic/AttributeCommonInfo.h:166
+   ? SpellingIndex
+   : calculateAttributeSpellingListIndex();
+  }

MaskRay wrote:
> calculateAttributeSpellingListIndex is defined in clangSema. This can cause 
> lib/libclangAST.so.10svn (-DBUILD_SHARED_LIBS=on) fail to link:
> 
> ```
> ld.lld: error: undefined symbol: 
> clang::AttributeCommonInfo::calculateAttributeSpellingListIndex() const   
> >>> referenced by AttributeCommonInfo.h:166 
> >>> (../tools/clang/include/clang/Basic/AttributeCommonInfo.h:166) 
> >>>   
> >>> tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/AttrImpl.cpp.o:(clang::AttributeCommonInfo::getAttributeSpellingListIndex()
> >>>  const)
> ```
+1 This fails builds with `-DBUILD_SHARED_LIBS=ON`. I tried to add `clangSema` 
as a dependent library to `clangAST`, but this creates several circular 
dependencies.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67368



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

[PATCH] D67253: clang-misexpect: a standalone tool for verifying the use of __builtin_expect with PGO data

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Layering feels weird here.
Can this be implemented as/limited to just a
`run-clang-misexpect.py` wrapper over clang itself?
That would be best IMHO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67253



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