[mlir][ods] Remove warning in `AttrOrTypeDef`

[Mogball]

Summary

This warning was added because using attribute or type assembly formats
with skipDefaultBuilders set could cause compilation errors, since the
required builder prototype may not necessarily be generated and would
need to be checked by hand. This patch removes the warning because a
warning that the generated C++ "might" not compile is not particularly
useful. Attempting to address the TODO (i.e. detect whether a builder of
the correct prototype is provided) would be fragile since it would not
be possible to account for implicit conversions, etc.

In general, ODS should not be emitting warnings in cases like these.

[rriddle]

Definitely agree we are getting a lot of false positives right now, which is moving the warning more into the "not helpful" territory. Do we have an idea of how many of these would go away after https://reviews.llvm.org/D129243 ? @wrengr Do you have any updates there?

[wrengr]

In D130102#3663119, @rriddle wrote:

Definitely agree we are getting a lot of false positives right now, which is moving the warning more into the "not helpful" territory. Do we have an idea of how many of these would go away after https://reviews.llvm.org/D129243 ? @wrengr Do you have any updates there?

I do have some updates for D129243. Unfortunately, I'm running into issues that are turning it into a rabbithole (even without doing anything more sophisticated re string matching). I'll give a more thorough discussion on that differential when I post the update later today.

Are there any false positives within the monorepo itself, or are they all in downstream repos? I didn't see any in the monorepo (else I wouldn't've landed the previous change), but I could've missed it. If they're in downstream repos, then yeah it'd be helpful to know if (the later-today version of) D129243 would be sufficient.

For the time being, I think it's fine to land this differential, though I'd like to retain a comment referring to https://github.com/llvm/llvm-project/issues/56415 so that readers have a guidepost for debugging and know where to go for further discussion/design

[Mogball]

Are there any false positives within the monorepo itself, or are they all in downstream repos? I didn't see any in the monorepo (else I wouldn't've landed the previous change), but I could've missed it. If they're in downstream repos, then yeah it'd be helpful to know if (the later-today version of) D129243 would be sufficient.

There's one in the monorepo (TestDialect) and there are a few downstream.

[Mogball]

Add a reference to the git issue

[rriddle]

mlir/lib/TableGen/AttrOrTypeDef.cpp

94

[Mogball]

Review comments

mlir/lib/TableGen/AttrOrTypeDef.cpp

94

Nice! didn't know about this.

[wrengr]

mlir/lib/TableGen/AttrOrTypeDef.cpp

94

ooh, very nice :)

[wrengr]

In D130102#3663593, @Mogball wrote:

There's one in the monorepo (TestDialect) and there are a few downstream.

What's the incantation to see this? When I run CMake, it doesn't seem to dump the warnings anywhere I can find...

mlir/lib/TableGen/AttrOrTypeDef.cpp

if (!parameters.empty() && !hasCppFormat && !hasDeclarativeFormat) {
                       "must specify either 'assemblyFormat' or "
                       "'hasCustomAssemblyFormat' when 'mnemonic' is set");
     }
   } else if (hasCppFormat || hasDeclarativeFormat) {
     PrintFatalError(getLoc(),
                     "'assemblyFormat' or 'hasCustomAssemblyFormat' can only be "
                     "used when 'mnemonic' is set");
   }
-  // Assembly format parser requires builders with the same prototype
-  // as the default-builders.
-  // TODO: attempt to detect when a custom builder matches the prototype.
-  if (hasDeclarativeFormat && skipDefaultBuilders()) {
-    PrintWarning(getLoc(),
-                 "using 'assemblyFormat' with 'skipDefaultBuilders=1' may "
-                 "result in C++ compilation errors");
-  }
   // Assembly format printer requires accessors to be generated.
   if (hasDeclarativeFormat && !genAccessors()) {
     PrintFatalError(getLoc(),
                     "'assemblyFormat' requires 'genAccessors' to be true");
   }
+  // TODO: Ensure that a suitable builder prototype can be generated:
+  // https://llvm.org/PR56415
 }
 
 Dialect AttrOrTypeDef::getDialect() const {
   auto *dialect = dyn_cast<llvm::DefInit>(def->getValue("dialect")->getValue());
   return Dialect(dialect ? dialect->getDef() : nullptr);
 }
 
 StringRef AttrOrTypeDef::getName() const { return def->getName(); }